[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