[FFmpeg-devel] [PATCH][RFC] variable frame sizes

[Eric Buehl]

> The code must not be exploitable.
> That is it must not be possible to execute arbitrary code by any
> intentionally created sequence of bytes.
> -> under no circumstances may a write happen to a address that is outside
> the intended array
>
> overflows in variables related to picture dimensions are likely not safe
> nor is randomly changing the output w/h



I believe the code in question is these section of lines:

> +            /* keep bands proportional to the frame size */
> +            ost->topBand    = MAKE_EVEN(ist->st->codec->height *
ost->original_topBand / ost->original_height);
> +            ost->bottomBand = MAKE_EVEN(ist->st->codec->height *
ost->original_bottomBand / ost->original_height);
> +            ost->leftBand   = MAKE_EVEN(ist->st->codec->width *
ost->original_leftBand / ost->original_width);
> +            ost->rightBand  = MAKE_EVEN(ist->st->codec->width *
ost->original_rightBand / ost->original_width);

The relation of original_*band and original_[width,height] is constrained by
the appropriate opt_frame_crop_* function such that frame_*Band < 0 and
(frame_*Band) >= frame_width.  If these constraints hold, I don't see how
the result could ever not fit within 32 bits (assuming
ist->st->codec->[height,width] is always a 32 bit int).

However, if one of these preconditions were to be changed, yes, it could
overflow.  As a sanity check, perhaps it should error out if this was
detected so as not to introduce a seemingly random change in output
height/width?  Something like assert(result <= INT_MAX && result > 0) before
the 64->32 cast?