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.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
