[FFmpeg-devel] libavutil/imgutils: UBSan nullptr-with-offset in av_image_fill_pointer

[Michael Niedermayer]

On Tue, Jul 07, 2020 at 05:41:11PM -0300, James Almer wrote:
> On 7/7/2020 5:07 PM, Brian Kim wrote:
> > On Tue, Jul 7, 2020 at 6:34 AM James Almer <jamrial at gmail.com> wrote:
> >>
> >> If i understand this right, you could easily solve it with just the
> >> following changes:
> >>
> >>> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> >>> index 7f9c1b632c..48a373db01 100644
> >>> --- a/libavutil/imgutils.c
> >>> +++ b/libavutil/imgutils.c
> >>> @@ -126,7 +126,8 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
> >>>
> >>>      if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
> >>>          desc->flags & FF_PSEUDOPAL) {
> >>> -        data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits words */
> >>> +        if (ptr)
> >>> +            data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits words */
> >>>          return size[0] + 256 * 4;
> >>>      }
> >>>
> >>> @@ -136,7 +137,8 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
> >>>      total_size = size[0];
> >>>      for (i = 1; i < 4 && has_plane[i]; i++) {
> >>>          int h, s = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
> >>> -        data[i] = data[i-1] + size[i-1];
> >>> +        if (data[i - 1])
> >>> +            data[i] = data[i - 1] + size[i - 1];
> >>>          h = (height + (1 << s) - 1) >> s;
> >>>          if (linesizes[i] > INT_MAX / h)
> >>>              return AVERROR(EINVAL);
> > 
> > Do we have to worry about backwards compatibility here? Some places
> > (e.g. libavcodec/decode.c:1497) have been using data to calculate the
> > sizes.
> 
> That code depended on undefined behavior, for both C and the
> av_image_fill_pointers() function. So IMO no, we don't need to worry
> about it.

If you break the size = data0 - data1 usecase then
you must bump the major abi version because if you dont then
distros will have things break and this is our own libs that break
because they use this before we fix it.

I hope iam wrong, of course because this is a mess 

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200708/3de72e3e/attachment.sig>