[FFmpeg-devel] [PATCH] avcodec/apedec: use int64_t for FFABS

Ronald S. Bultje rsbultje at gmail.com
Mon Oct 12 17:23:29 CEST 2015


Hi,

On Fri, Oct 9, 2015 at 1:48 PM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
wrote:

> res, absres are currently int's, which on most platforms is 32 bits.
> Unfortunately, data is untrusted, and on line 1267 res is manipulated
> with data. Thus, res can take on INT32_MIN/INT_MIN with crafted data,
> making FFABS on line 1282 unsafe.
>
> Once again, using FFNABS will make it less readable: logic is less
> clear, diff is bigger, and there is scope for mistakes during the
> expression rewrites.
>
> Tested with FATE.
>
> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> ---
>  libavcodec/apedec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index fcccfbe..e46558e 100644
> --- a/libavcodec/apedec.c
> +++ b/libavcodec/apedec.c
> @@ -1254,8 +1254,8 @@ static void init_filter(APEContext *ctx, APEFilter
> *f, int16_t *buf, int order)
>  static void do_apply_filter(APEContext *ctx, int version, APEFilter *f,
>                              int32_t *data, int count, int order, int
> fracbits)
>  {
> -    int res;
> -    int absres;
> +    int64_t res;
> +    int64_t absres;
>
>      while (count--) {
>          /* round fixedpoint scalar product */
> --
> 2.6.1


If you want to take this approach, can't you just cast the input argument
to FFABS instead of this?

You could also make absres unsigned, no?

Ronald


More information about the ffmpeg-devel mailing list