| Message ID | 011aa19c-d4dd-4dec-b7e6-3243187a215f@jkqxz.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] cbs_vp9: Ensure that the two superframe_header instances are identical | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
On Sun, Aug 11, 2024 at 07:17:25PM +0100, Mark Thompson wrote: > Fixes: use of uninitialized value > Fixes: 70907/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_METADATA_fuzzer-6339363208757248 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > --- > On 11/08/2024 19:05, Mark Thompson wrote: > > The correct fix therefore would be to constrain the second read values to be identical to the first, not to introduce new syntax elements not in the standard to cover the invalid case. > > Like this. (Marked in the same way as your suggested patch based on my assumption that it fixes the problem - please check.) > > Trace output is correct in the normal case, and catches the error in the bad one: > > [trace_headers @ 0x55a0f5decb40] Packet: 11971 bytes, pts 366, dts 366. > [trace_headers @ 0x55a0f5decb40] Superframe Index > [trace_headers @ 0x55a0f5decb40] 0 superframe_marker 110 = 6 > [trace_headers @ 0x55a0f5decb40] 3 bytes_per_framesize_minus_1 01 = 1 > [trace_headers @ 0x55a0f5decb40] 5 frames_in_superframe_minus_1 001 = 1 > [trace_headers @ 0x55a0f5decb40] 8 frame_sizes[0] 1011110000101110 = 11964 > [trace_headers @ 0x55a0f5decb40] 24 frame_sizes[1] 0000000100000000 = 1 > [trace_headers @ 0x55a0f5decb40] 40 superframe_marker 110 = 6 > [trace_headers @ 0x55a0f5decb40] 43 bytes_per_framesize_minus_1 01 = 1 > [trace_headers @ 0x55a0f5decb40] 45 frames_in_superframe_minus_1 001 = 1 > > or > > [trace_headers @ 0x555af04d7b40] Packet: 11971 bytes, pts 366, dts 366. > [trace_headers @ 0x555af04d7b40] Superframe Index > [trace_headers @ 0x555af04d7b40] 0 superframe_marker 110 = 6 > [trace_headers @ 0x555af04d7b40] 3 bytes_per_framesize_minus_1 01 = 1 > [trace_headers @ 0x555af04d7b40] 5 frames_in_superframe_minus_1 001 = 1 > [trace_headers @ 0x555af04d7b40] 8 frame_sizes[0] 1011110000101110 = 11964 > [trace_headers @ 0x555af04d7b40] 24 frame_sizes[1] 0000000100000000 = 1 > [trace_headers @ 0x555af04d7b40] 40 superframe_marker 110 = 6 > [trace_headers @ 0x555af04d7b40] 43 bytes_per_framesize_minus_1 10 = 2 > [trace_headers @ 0x555af04d7b40] bytes_per_framesize_minus_1 out of range: 2, but must be in [1,1]. > [vost#0:0/copy @ 0x555af0538400] Error applying bitstream filters to a packet: Invalid data found when processing input > > Thanks, > > - Mark > > libavcodec/cbs_vp9_syntax_template.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) If its intended that this will discard cases where they differ then your patch is probably ok thx [...]
diff --git a/libavcodec/cbs_vp9_syntax_template.c b/libavcodec/cbs_vp9_syntax_template.c index 2f08eccf18..5ed3c700dc 100644 --- a/libavcodec/cbs_vp9_syntax_template.c +++ b/libavcodec/cbs_vp9_syntax_template.c @@ -421,9 +421,14 @@ static int FUNC(superframe_index)(CodedBitstreamContext *ctx, RWContext *rw, frame_sizes[i], 1, i); } - f(3, superframe_marker); - f(2, bytes_per_framesize_minus_1); - f(3, frames_in_superframe_minus_1); + // Second instance of the superframe header must be identical + // to the first. + fixed(3, superframe_marker, + current->superframe_marker); + fixed(2, bytes_per_framesize_minus_1, + current->bytes_per_framesize_minus_1); + fixed(3, frames_in_superframe_minus_1, + current->frames_in_superframe_minus_1); return 0; }
Fixes: use of uninitialized value Fixes: 70907/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_METADATA_fuzzer-6339363208757248 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg --- On 11/08/2024 19:05, Mark Thompson wrote: > The correct fix therefore would be to constrain the second read values to be identical to the first, not to introduce new syntax elements not in the standard to cover the invalid case. Like this. (Marked in the same way as your suggested patch based on my assumption that it fixes the problem - please check.) Trace output is correct in the normal case, and catches the error in the bad one: [trace_headers @ 0x55a0f5decb40] Packet: 11971 bytes, pts 366, dts 366. [trace_headers @ 0x55a0f5decb40] Superframe Index [trace_headers @ 0x55a0f5decb40] 0 superframe_marker 110 = 6 [trace_headers @ 0x55a0f5decb40] 3 bytes_per_framesize_minus_1 01 = 1 [trace_headers @ 0x55a0f5decb40] 5 frames_in_superframe_minus_1 001 = 1 [trace_headers @ 0x55a0f5decb40] 8 frame_sizes[0] 1011110000101110 = 11964 [trace_headers @ 0x55a0f5decb40] 24 frame_sizes[1] 0000000100000000 = 1 [trace_headers @ 0x55a0f5decb40] 40 superframe_marker 110 = 6 [trace_headers @ 0x55a0f5decb40] 43 bytes_per_framesize_minus_1 01 = 1 [trace_headers @ 0x55a0f5decb40] 45 frames_in_superframe_minus_1 001 = 1 or [trace_headers @ 0x555af04d7b40] Packet: 11971 bytes, pts 366, dts 366. [trace_headers @ 0x555af04d7b40] Superframe Index [trace_headers @ 0x555af04d7b40] 0 superframe_marker 110 = 6 [trace_headers @ 0x555af04d7b40] 3 bytes_per_framesize_minus_1 01 = 1 [trace_headers @ 0x555af04d7b40] 5 frames_in_superframe_minus_1 001 = 1 [trace_headers @ 0x555af04d7b40] 8 frame_sizes[0] 1011110000101110 = 11964 [trace_headers @ 0x555af04d7b40] 24 frame_sizes[1] 0000000100000000 = 1 [trace_headers @ 0x555af04d7b40] 40 superframe_marker 110 = 6 [trace_headers @ 0x555af04d7b40] 43 bytes_per_framesize_minus_1 10 = 2 [trace_headers @ 0x555af04d7b40] bytes_per_framesize_minus_1 out of range: 2, but must be in [1,1]. [vost#0:0/copy @ 0x555af0538400] Error applying bitstream filters to a packet: Invalid data found when processing input Thanks, - Mark libavcodec/cbs_vp9_syntax_template.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)