| Message ID | 8088a842-01e8-1efb-90fc-febaa4b34ae6@googlemail.com |
|---|---|
| State | Superseded |
| Headers | show |
> > + expected_len = FFMIN(expected_len, uncompressed_size); > + > dest_len = expected_len; > > > Hello, in what case expected_len can be > to uncompress_size ? Did you have a test sample ? Martin
Hi, On 16.11.2016 21:29, Martin Vignali wrote: > in what case expected_len can be > to uncompress_size ? td->xsize = 800 td->ysize = 16 s->current_channel_offset = 5 td->channel_line_size = s->current_channel_offset * td->xsize = 4000 uncompressed_size = td->channel_line_size * td->ysize = 64000 s->nb_channels = 3 p s->channels[0].pixel_type = EXR_HALF expected_len = td->xsize * td->ysize * 2 = 25600 p s->channels[1].pixel_type = EXR_UINT expected_len = expected_len + td->xsize * td->ysize * 4 = 76800 p s->channels[2].pixel_type = EXR_HALF expected_len = expected_len + td->xsize * td->ysize * 2 = 102400 => expected_len > uncompressed_size > Did you have a test sample ? Yes. Best regards, Andreas
> > td->xsize = 800 > td->ysize = 16 > s->current_channel_offset = 5 > td->channel_line_size = s->current_channel_offset * td->xsize = 4000 > uncompressed_size = td->channel_line_size * td->ysize = 64000 > > s->nb_channels = 3 > > p s->channels[0].pixel_type = EXR_HALF > expected_len = td->xsize * td->ysize * 2 = 25600 > > p s->channels[1].pixel_type = EXR_UINT > expected_len = expected_len + td->xsize * td->ysize * 4 = 76800 > > p s->channels[2].pixel_type = EXR_HALF > expected_len = expected_len + td->xsize * td->ysize * 2 = 102400 > > => expected_len > uncompressed_size > > > Did you have a test sample ? > > Yes. > > Thanks for your explanation. I think the trouble come from another place The current channel offset is not correct for uint32 (because it use 1<<0 = 1 instead of 4 for uint32) Some months ago i propose a patch who was not accepted for uint32 support in exr. But the current channel offset is correctly calculate in this patch. Conversation can be found here : http://ffmpeg.org/pipermail/ffmpeg-devel/2016-April/192527.html The interested part is that : - s->current_channel_offset += 1 << current_pixel_type; + if (current_pixel_type == EXR_HALF) { + s->current_channel_offset += 2; + } else {/* Float or UINT32 */ + s->current_channel_offset += 4; + } I think it's better to fix the uncompress size calculation. Martin
On 16.11.2016 22:16, Martin Vignali wrote: > I think the trouble come from another place > The current channel offset is not correct for uint32 (because it use 1<<0 = 1 instead of 4 for uint32) > > Some months ago i propose a patch who was not accepted for uint32 support in exr. > But the current channel offset is correctly calculate in this patch. > > Conversation can be found here : > http://ffmpeg.org/pipermail/ffmpeg-devel/2016-April/192527.html > > The interested part is that : > - s->current_channel_offset += 1 << current_pixel_type; > + if (current_pixel_type == EXR_HALF) { > + s->current_channel_offset += 2; > + } else {/* Float or UINT32 */ > + s->current_channel_offset += 4; > + } > > > I think it's better to fix the uncompress size calculation. Yes, that is a better fix. Can you send a proper patch for this? Best regards, Andreas
> > Yes, that is a better fix. Can you send a proper patch for this? > > Best regards, > Andreas > > Yes i will send a patch for this. Martin
diff --git a/libavcodec/exr.c b/libavcodec/exr.c index c250eea..54869d2 100644 --- a/libavcodec/exr.c +++ b/libavcodec/exr.c @@ -841,6 +841,8 @@ static int pxr24_uncompress(EXRContext *s, const uint8_t *src, } } + expected_len = FFMIN(expected_len, uncompressed_size); + dest_len = expected_len; if (uncompress(td->tmp, &dest_len, src, compressed_size) != Z_OK) {
This fixes crashes due to pointer corruption caused by invalid writes. The problem was introduced in commit 03152e74dfdc7f438cb4a10402c4de744e807e22. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavcodec/exr.c | 2 ++ 1 file changed, 2 insertions(+)