Usage-of-uninitialized value

[Andrew Bao]

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];

2. arbitrary memory write, leading to crash or code execution

BUCKET(queue->available++) = buf;

3. Change control flow

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

4. 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

[mkver]

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.