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

[Michael Niedermayer]

On Fri, Feb 01, 2019 at 06:16:59PM -0800, chcunningham wrote:
> Unsigned types match the isobmff spec.
> ---
>  libavformat/isom.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index e629663949..8e0d8355b3 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -59,9 +59,9 @@ typedef struct MOVStts {
>  } MOVStts;
>  
>  typedef struct MOVStsc {
> -    int first;
> -    int count;
> -    int id;
> +    unsigned int first;
> +    unsigned int count;
> +    unsigned int id;
>  } MOVStsc;

Is this change needed by some valid file ?

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

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190202/9edab6cd/attachment.sig>