[FFmpeg-devel] [PATCH] avformat/isom.h: use usnigned types in MOVStsc

Chris Cunningham chcunningham at chromium.org
Mon Feb 4 23:48:49 EET 2019


On Sat, Feb 2, 2019 at 3:37 AM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> Is this change needed by some valid file ?
>

No, just a drive by fix since I happened to be looking at these types and
the spec.


> The change taken on its own is probably correct but its increasing the
> range
> of values without actually adding support for that thus quite possibly
> introducing bugs.
>
> In case of the id, we use signed int for the entries this indexes into,
> so the extended range is not going to work.  also wonder if billions
> of STSD entries in a stream will work when more than 1 cause problems
> already.
>
> Another obvious issue is that currently we scan this table for negative
> values and eliminate them but when this is made unsigned suddenly
> larger values pass through. This has the potential to introduce
> integer overflow bugs in later stages. More so unsigned overflows dont
> show up in asan and these first/count values might affect array indexes
> iam not saying theres a bug here just that its the set of circunstances
> that is fertile for such bugs.
>
> variables related to indexes or counts always require extra scrutiny
> when their type is changed
>

I really appreciate the scrutiny. Given I don't have a file that needs
this, happy to abandon this patch.

Chris


More information about the ffmpeg-devel mailing list