Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#9314 closed defect (invalid)

Usage-of-uninitialized value

Reported by: Andrew Bao Owned by:
Priority: normal Component: undetermined
Version: unspecified Keywords:
Cc: Andrew Bao Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
In libavcodec/vorbisec.c, function vorbis_encode_init():
It partially initializes the structure vorbis_enc_context, leading to the memory pointed by venc in an uninitialized state.

Details:

In function vorbis_encode_init(), it will initialzie structure vorbis_enc_context by calling create_vorbis_context(). However, it initalizes all the field except struct FFBufQueue bufqueue.

struct FFBufQueue {
 50     AVFrame *queue[FF_BUFQUEUE_SIZE];
 51     unsigned short head;
 52     unsigned short available; /**< number of available buffers */
 53 };

Then bufqueue will be used in several places:

move_audio():

1073         cur = ff_bufqueue_get(&venc->bufqueue);

vorbis_encode_frame():

1107         ff_bufqueue_add(avctx, &venc->bufqueue, clone);
1112         need_more = venc->bufqueue.available * avctx->frame_size < frame_size;
1119         if (venc->bufqueue.available * avctx->frame_size < frame_size) {
1120             int frames_needed = (frame_size/avctx->frame_size) - venc->bufqueue.available;
1128         ff_bufqueue_add(avctx, &venc->bufqueue, empty);



Since venc->bufqueue are not initialized, the fields

unsigned short head;
unsigned short available;

are undecidable. It might be set to zero or a random value inherited from the previous stack.

In this case, if the variable available and head are set to random values, it will cause the program:

  1. array read out of bound, leading to crash
   AVFrame *ret = queue->queue[queue->head];
  1. arbitrary memory write, leading to crash or code execution
BUCKET(queue->available++) = buf;
  1. Change control flow

if (venc->bufqueue.available * avctx->frame_size < frame_size) {

  1. change variable, running out of memory :
int frames_needed = (frame_size/avctx->frame_size) - venc->bufqueue.available;

 for (i = 0; i < frames_needed; i++) {
              AVFrame *empty = spawn_empty_frame(avctx, venc->channels);

recommend fix:
initialized struct FFBufQueue bufqueue during initialization

Change History (2)

comment:1 by mkver, 12 months ago

Resolution: invalid
Status: newclosed

How did you check that these values are uninitialized? Every codec's private context (like the vorbis_enc_context struct for the Vorbis encoder) is zero-initialized during its allocation (yes, this thing lives on the heap, so it does not inherit anything from the "previous stack"), so it is not uninitialized.

comment:2 by Carl Eugen Hoyos, 12 months ago

Component: avcodecundetermined
Keywords: Usage-of-uninitialized value bug removed
Priority: importantnormal
Note: See TracTickets for help on using tickets.