| Message ID | 20190416144834.20300-4-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | e627113329da7c7c21b00562515a1f11437716f3 |
| Headers | show |
Michael Niedermayer: > Fixes: Timeout (21sec -> 0.6sec) > Fixes: 14134/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5768371078955008 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/jpeg2000dec.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > index 9198cf87cb..62b0b1f9b7 100644 > --- a/libavcodec/jpeg2000dec.c > +++ b/libavcodec/jpeg2000dec.c > @@ -832,15 +832,21 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n) > static uint8_t get_plt(Jpeg2000DecoderContext *s, int n) > { > int i; > + int v; > > av_log(s->avctx, AV_LOG_DEBUG, > "PLT marker at pos 0x%X\n", bytestream2_tell(&s->g) - 4); > > + if (n < 4) > + return AVERROR_INVALIDDATA; > + > /*Zplt =*/ bytestream2_get_byte(&s->g); > > for (i = 0; i < n - 3; i++) { > - bytestream2_get_byte(&s->g); > + v = bytestream2_get_byte(&s->g); > } > + if (v & 0x80) > + return AVERROR_INVALIDDATA; > > return 0; > } > get_tlm should probably return int as uint8_t can't hold the AVERROR_INVALIDDATA. gcc gives Woverflow warnings because of that. Currently, the return value of get_tlm is always >= 0 and so is the return value of jpeg2000_read_main_headers and of jpeg2000_decode_frame if one runs into the AVERROR_INVALIDDATA checks. (I didn't test it, just took a quick glimpse at the code.) - Andreas
Andreas Rheinhardt: > Michael Niedermayer: >> Fixes: Timeout (21sec -> 0.6sec) >> Fixes: 14134/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5768371078955008 >> >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavcodec/jpeg2000dec.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c >> index 9198cf87cb..62b0b1f9b7 100644 >> --- a/libavcodec/jpeg2000dec.c >> +++ b/libavcodec/jpeg2000dec.c >> @@ -832,15 +832,21 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n) >> static uint8_t get_plt(Jpeg2000DecoderContext *s, int n) >> { >> int i; >> + int v; >> >> av_log(s->avctx, AV_LOG_DEBUG, >> "PLT marker at pos 0x%X\n", bytestream2_tell(&s->g) - 4); >> >> + if (n < 4) >> + return AVERROR_INVALIDDATA; >> + >> /*Zplt =*/ bytestream2_get_byte(&s->g); >> >> for (i = 0; i < n - 3; i++) { >> - bytestream2_get_byte(&s->g); >> + v = bytestream2_get_byte(&s->g); >> } >> + if (v & 0x80) >> + return AVERROR_INVALIDDATA; >> >> return 0; >> } >> > get_tlm should probably return int as uint8_t can't hold the > AVERROR_INVALIDDATA. gcc gives Woverflow warnings because of that. > > Currently, the return value of get_tlm is always >= 0 and so is the > return value of jpeg2000_read_main_headers and of > jpeg2000_decode_frame if one runs into the AVERROR_INVALIDDATA checks. > (I didn't test it, just took a quick glimpse at the code.) > > - Andreas > get_plt, not get_tlm. Sorry. - Andreas
On Thu, Apr 25, 2019 at 04:34:00PM +0000, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: Timeout (21sec -> 0.6sec) > > Fixes: 14134/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5768371078955008 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/jpeg2000dec.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > index 9198cf87cb..62b0b1f9b7 100644 > > --- a/libavcodec/jpeg2000dec.c > > +++ b/libavcodec/jpeg2000dec.c > > @@ -832,15 +832,21 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n) > > static uint8_t get_plt(Jpeg2000DecoderContext *s, int n) > > { > > int i; > > + int v; > > > > av_log(s->avctx, AV_LOG_DEBUG, > > "PLT marker at pos 0x%X\n", bytestream2_tell(&s->g) - 4); > > > > + if (n < 4) > > + return AVERROR_INVALIDDATA; > > + > > /*Zplt =*/ bytestream2_get_byte(&s->g); > > > > for (i = 0; i < n - 3; i++) { > > - bytestream2_get_byte(&s->g); > > + v = bytestream2_get_byte(&s->g); > > } > > + if (v & 0x80) > > + return AVERROR_INVALIDDATA; > > > > return 0; > > } > > > get_tlm should probably return int as uint8_t can't hold the > AVERROR_INVALIDDATA. gcc gives Woverflow warnings because of that. > > Currently, the return value of get_tlm is always >= 0 and so is the > return value of jpeg2000_read_main_headers and of > jpeg2000_decode_frame if one runs into the AVERROR_INVALIDDATA checks. > (I didn't test it, just took a quick glimpse at the code.) will fix thanks [...]
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c index 9198cf87cb..62b0b1f9b7 100644 --- a/libavcodec/jpeg2000dec.c +++ b/libavcodec/jpeg2000dec.c @@ -832,15 +832,21 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n) static uint8_t get_plt(Jpeg2000DecoderContext *s, int n) { int i; + int v; av_log(s->avctx, AV_LOG_DEBUG, "PLT marker at pos 0x%X\n", bytestream2_tell(&s->g) - 4); + if (n < 4) + return AVERROR_INVALIDDATA; + /*Zplt =*/ bytestream2_get_byte(&s->g); for (i = 0; i < n - 3; i++) { - bytestream2_get_byte(&s->g); + v = bytestream2_get_byte(&s->g); } + if (v & 0x80) + return AVERROR_INVALIDDATA; return 0; }
Fixes: Timeout (21sec -> 0.6sec) Fixes: 14134/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5768371078955008 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/jpeg2000dec.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)