Opened 5 years ago

Closed 5 years ago

#8271 closed defect (invalid)

A use-after-free bug in libavcodec/cuvid.c

Reported by: wurongxin Owned by:
Priority: critical Component: avcodec
Version: 3.4.6 Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
How to reproduce:

% ffmpeg -i input ... output
ffmpeg version
built on ...

Patches should be submitted to the ffmpeg-devel mailing list and not this bug tracker.

In the source file https://github.com/FFmpeg/FFmpeg/blob/release/3.4/libavcodec/cuvid.c, in the function "cuvid_output_frame", at Line 495, the variable "pkt->buf->buffer" could be freed when calling to "cuvid_decode_packet". At Line 496, the variable "pkt->buf->buffer" could be freed again when calling to "av_packet_unref" (See the below code).

https://github.com/FFmpeg/FFmpeg/blob/release/3.4/libavcodec/cuvid.c

473. static int cuvid_output_frame(AVCodecContext *avctx, AVFrame *frame)
474. {
...
495.         ret = cuvid_decode_packet(avctx, &pkt);
496.         av_packet_unref(&pkt);

To see how the function call to "cuvid_decode_packet" freed the variable "pkt->buf->buffer", please read the following code snippet. In the function "cuvid_decode_packet", at Line 403, it calls to the function "av_packet_ref", where filter_packet.buf->buffer is assigned with avpkt->buf->buffer (More details can be refered to the implementation of av_package_ref as below). Note that, filter_packet.buf->buffer is a shallow copy of avpkt->buf->buffer. At Line 410, it will call the function "av_packet_unref", and free filter_packet.buf->buffer. Thus, the variable "avpkt->buf->buffer" is also freed. Although filter_packet.buf->buffer is assigned with null pointer when calling to av_packet_unref, avpkt->buf->buffer is not assigned with null pointer. Then, the dangling pointer avpkt->buf->buffer is returned back to its caller "cuvid_output_frame".

https://github.com/FFmpeg/FFmpeg/blob/release/3.4/libavcodec/cuvid.c

383. static int cuvid_decode_packet(AVCodecContext *avctx, const AVPacket *avpkt)
384. {
	...
403.        if ((ret = av_packet_ref(&filter_packet, avpkt)) < 0) {
404.             av_log(avctx, AV_LOG_ERROR, "av_packet_ref failed\n");
405.             return ret;
406.        }
407.
408.        if ((ret = av_bsf_send_packet(ctx->bsf, &filter_packet)) < 0) {
409.            av_log(avctx, AV_LOG_ERROR, "av_bsf_send_packet failed\n");
410.            av_packet_unref(&filter_packet);
411.            return ret;
412.        }


https://github.com/FFmpeg/FFmpeg/blob/release/3.4/libavcodec/avpacket.c
627. int av_packet_ref(AVPacket *dst, const AVPacket *src)
628. {
        ...
635.    if (!src->buf) {
            ...
643.    } else {
644.        dst->buf = av_buffer_ref(src->buf);
            ...
650.    }	    

https://github.com/FFmpeg/FFmpeg/blob/release/3.4/libavutil/buffer.c

93. AVBufferRef *av_buffer_ref(AVBufferRef *buf)
94. {
95.     AVBufferRef *ret = av_mallocz(sizeof(*ret));
96.
97.     if (!ret)
98.         return NULL;
99.
100.    *ret = *buf;
101.
102.    atomic_fetch_add_explicit(&buf->buffer->refcount, 1, memory_order_relaxed);
103.
104.    return ret;
105.}


Change History (1)

comment:1 by Hendrik, 5 years ago

Resolution: invalid
Status: newclosed

Packets and Buffers are reference counted. As long as av_packet_* and av_buffer_* API is used everywhere, then there is no issues.

Note: See TracTickets for help on using tickets.