| Message ID | 20230608142637.45033-6-leo.izen@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | JPEG XL Animation Changes | 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 |
Quoting Leo Izen (2023-06-08 16:26:37) > This should avoid overrunning buffers with jxlp boxes if the size is > zero or if the size is so small the box is invalid. > > Signed-off-by: Leo Izen <leo.izen@gmail.com> > --- > libavformat/jpegxl_anim_dec.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c > index 6ea6c46d8f..c9e4dcd5fc 100644 > --- a/libavformat/jpegxl_anim_dec.c > +++ b/libavformat/jpegxl_anim_dec.c > @@ -76,8 +76,14 @@ static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int inp > tag = AV_RL32(b); > b += 4; > if (tag == MKTAG('j', 'x', 'l', 'p')) { > + if (b - input_buffer >= input_len - 4) > + break; > b += 4; > - size -= 4; > + if (size) { > + if (size < 4) > + return AVERROR_INVALIDDATA; > + size -= 4; > + } This looks like it should be using bytestream2. Is there a good reason it is not?
On 6/8/23 22:30, Anton Khirnov wrote: > Quoting Leo Izen (2023-06-08 16:26:37) >> This should avoid overrunning buffers with jxlp boxes if the size is >> zero or if the size is so small the box is invalid. >> >> Signed-off-by: Leo Izen <leo.izen@gmail.com> >> --- >> libavformat/jpegxl_anim_dec.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c >> index 6ea6c46d8f..c9e4dcd5fc 100644 >> --- a/libavformat/jpegxl_anim_dec.c >> +++ b/libavformat/jpegxl_anim_dec.c >> @@ -76,8 +76,14 @@ static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int inp >> tag = AV_RL32(b); >> b += 4; >> if (tag == MKTAG('j', 'x', 'l', 'p')) { >> + if (b - input_buffer >= input_len - 4) >> + break; >> b += 4; >> - size -= 4; >> + if (size) { >> + if (size < 4) >> + return AVERROR_INVALIDDATA; >> + size -= 4; >> + } > > This looks like it should be using bytestream2. Is there a good reason > it is not? > No particular reason, I'll send an updated patch using it. Also, I pushed the first three patches that michaelni authored, but not the 4th or this one. - Leo Izen
diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c index 6ea6c46d8f..c9e4dcd5fc 100644 --- a/libavformat/jpegxl_anim_dec.c +++ b/libavformat/jpegxl_anim_dec.c @@ -76,8 +76,14 @@ static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int inp tag = AV_RL32(b); b += 4; if (tag == MKTAG('j', 'x', 'l', 'p')) { + if (b - input_buffer >= input_len - 4) + break; b += 4; - size -= 4; + if (size) { + if (size < 4) + return AVERROR_INVALIDDATA; + size -= 4; + } } if (tag == MKTAG('j', 'x', 'l', 'c') || tag == MKTAG('j', 'x', 'l', 'p')) {
This should avoid overrunning buffers with jxlp boxes if the size is zero or if the size is so small the box is invalid. Signed-off-by: Leo Izen <leo.izen@gmail.com> --- libavformat/jpegxl_anim_dec.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)