| Message ID | 20230802000135.26482-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/jpeg2000htdec: Avoid freeing uninitialized pointers in ff_jpeg2000_decode_htj2k() | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > > Code should make more sense now > > Fixes: out of array access > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-6627570448465920 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/jpeg2000htdec.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c > index 2b082b3b2f..3985783f3a 100644 > --- a/libavcodec/jpeg2000htdec.c > +++ b/libavcodec/jpeg2000htdec.c > @@ -159,21 +159,14 @@ static int jpeg2000_bitbuf_refill_backwards(StateVars *buffer, const uint8_t *ar > */ > position -= 4; Can't we get rid of this line and the comment above, and instead replace `int32_t position = buffer->pos;` with `int32_t position = buffer->pos - 4;`? LGTM otherwise. > > - tmp = AV_RB32(&array[position + 1]); > - > - if (buffer->pos < 4){ > - /* mask un-needed bits if we are close to input end */ > - uint64_t mask = (1ull << (buffer->pos + 1) * 8) - 1; > - tmp &= mask; > - } > - > /** > * Unstuff bits. Load a temporary byte, which precedes the position we > * currently at, to ensure that we can also un-stuff if the stuffed bit is > * the bottom most bits. > */ > - tmp <<= 8; > - tmp |= array[buffer->pos + 1]; > + > + for(int i = FFMAX(0, position + 1); i <= buffer->pos + 1; i++) > + tmp = 256*tmp + array[i]; > > if ((tmp & 0x7FFF000000) > 0x7F8F000000) { > tmp &= 0x7FFFFFFFFF; > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Fri, Aug 04, 2023 at 06:41:24PM -0700, Pierre-Anthony Lemieux wrote: > On Tue, Aug 1, 2023 at 5:02 PM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > Code should make more sense now > > > > Fixes: out of array access > > Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-6627570448465920 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/jpeg2000htdec.c | 13 +++---------- > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > > diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c > > index 2b082b3b2f..3985783f3a 100644 > > --- a/libavcodec/jpeg2000htdec.c > > +++ b/libavcodec/jpeg2000htdec.c > > @@ -159,21 +159,14 @@ static int jpeg2000_bitbuf_refill_backwards(StateVars *buffer, const uint8_t *ar > > */ > > position -= 4; > > Can't we get rid of this line and the comment above, and instead > replace `int32_t position = buffer->pos;` with `int32_t position = > buffer->pos - 4;`? yes > > LGTM otherwise. will apply with the suggested change thx [...]
diff --git a/libavcodec/jpeg2000htdec.c b/libavcodec/jpeg2000htdec.c index 2b082b3b2f..3985783f3a 100644 --- a/libavcodec/jpeg2000htdec.c +++ b/libavcodec/jpeg2000htdec.c @@ -159,21 +159,14 @@ static int jpeg2000_bitbuf_refill_backwards(StateVars *buffer, const uint8_t *ar */ position -= 4; - tmp = AV_RB32(&array[position + 1]); - - if (buffer->pos < 4){ - /* mask un-needed bits if we are close to input end */ - uint64_t mask = (1ull << (buffer->pos + 1) * 8) - 1; - tmp &= mask; - } - /** * Unstuff bits. Load a temporary byte, which precedes the position we * currently at, to ensure that we can also un-stuff if the stuffed bit is * the bottom most bits. */ - tmp <<= 8; - tmp |= array[buffer->pos + 1]; + + for(int i = FFMAX(0, position + 1); i <= buffer->pos + 1; i++) + tmp = 256*tmp + array[i]; if ((tmp & 0x7FFF000000) > 0x7F8F000000) { tmp &= 0x7FFFFFFFFF;
Code should make more sense now Fixes: out of array access Fixes: 58299/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-6627570448465920 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/jpeg2000htdec.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)