[FFmpeg-devel] [PATCH] lavc/vvc: Prevent OOB access in subpic_tiles

[Nuo Mi]

On Sat, Aug 24, 2024 at 5:01 PM Frank Plowman <post at frankplowman.com> wrote:

> Hi,
>
> Thanks for your review.
>
> On 24/08/2024 04:40, Nuo Mi wrote:
> > Hi Frank,
> > thank you for the patch
> > On Fri, Aug 23, 2024 at 7:45 PM Frank Plowman <post at frankplowman.com>
> wrote:
> >
> >> The previous logic relied on the subpicture boundaries coinciding with
> >> the tile boundaries.  Per 6.3.1 of H.266 (V3), vertical subpicture
> >> boundaries are always tile boundaries however the same cannot be said
> >> for horizontal subpicture boundaries.
> >
> > From the spec:
> > "One or both of the following conditions shall be fulfilled for each
> > subpicture and tile:
> >
> > – All CTUs in a subpicture belong to the same tile.
> >
> > – All CTUs in a tile belong to the same subpicture."
> >
> > This suggests that the subpicture boundary coincides with a tile
> boundary,
> > right?
>
> Not always.  As an example, consider a picture with only a single tile
> but split into two subpictures, one atop the other:
>
>       Tiles       Subpics       Slices
>     +-------+    +-------+    +-------+
>     |       |    |       |    |       |
>     |       |    +-------+    +-------+
>     |       |    |       |    |       |
>     +-------+    +-------+    +-------+
>
> The first of the conditions is met for both subpics: all CTUs of either
> subpic belong to the same tile.  It is clear to see, however, that the
> bottom boundary of the top subpicture and the top boundary of the bottom
> subpicture do not coincide with a tile boundary.  In conjunction with
> the restrictions on subpic and slice layout and those on slice and tile
> layout, you get a guarantee that the vertical boundaries of subpics
> coincide with vertical boundaries of tiles, but there is no guaranteed
> relationship between the horizontal boundaries of subpics and tile
> boundaries.
>
oh, thank you for the explanation.
Another code may assume subpic coincides with tile. it's my bad.
We can merge your patch first and fix another code when we have a valid clip

>
> >> Furthermore, it is possible to
> >> construct an illegal bitstream where vertical subpicture boundaries are
> >> not coincident with tile boundaries.  In these cases, the condition of
> >> the while loop would never be satisfied resulting in an OOB read on
> >> col_bd/row_bd.
> >>
> > Can we implement early checks to reject invalid streams?
> > If the picture boundaries are incorrect, it indicates a serious error in
> > the bitstream.
> >
>
> My first attempt at solving this problem took this approach.  I found
> that it was not trivial to write an algorithm which ensures all the
> relationships set out in 6.3.1.  As far as I can tell, the restrictions
> are only ever set out in prose in that section and never as more formal
> requirements on bitstream conformance.  Additionally, these checks
> appear to be missing from VTM, so there is no reference implementation
> to refer to (aside: VTM is quite happy to encode and decode bitstreams
> with invalid partitioning layouts).  As a result, I decided to take the
> approach of identifying where the relationships were used and making
> changes there instead.
>
  Thank you. Sometimes a full check isn't feasible, so we can only do our
best. If it's too difficult or the cost is too high, we can leave it as
is.

>
> Even if the partitioning structure were validated elsewhere however, the
> change to the horizontal boundaries would still be needed as set out
> above.  I also think the change to the vertical boundaries does no harm
> and serves to prevent an OOB access in the case that validation fails,
> however unlikely that may be.
>
> --
> All the best,
> Frank
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>