[vlc-devel] [PATCH] demux/asf: fix 17580: prevent overflow leading to crash

[Rémi Denis-Courmont]

Le tiistaina 1. marraskuuta 2016, 2.20.26 EET Filip Roséen a écrit :
> Given that the previous implementation assigned the return-value of
> vlc_stream_Peek to a size_t, the value would wrap around on error
> (since vlc_stream_Peek returns -1), rendering the "< 78" somewhat
> useless (when an error occurs).
> 
> These changes change the type of i_peek to correspond to that of
> vlc_stream_Peek, while also making sure that we error before calling
> the function if the object size is larger than SSIZE_MAX (meaning that
> we cannot peek).
> 
> fixes #17580
> ---
>  modules/demux/asf/libasf.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/modules/demux/asf/libasf.c b/modules/demux/asf/libasf.c
> index ed782bd..b303ce5 100644
> --- a/modules/demux/asf/libasf.c
> +++ b/modules/demux/asf/libasf.c
> @@ -25,6 +25,8 @@
>  # include "config.h"
>  #endif
> 
> +#include <limits.h>
> +
>  #include <vlc_demux.h>
>  #include <vlc_charset.h>          /* FromCharset */
> 
> @@ -528,9 +530,17 @@ static void ASF_FreeObject_header_extension(
> asf_object_t *p_obj ) static int ASF_ReadObject_stream_properties( stream_t
> *s, asf_object_t *p_obj ) {
>      asf_object_stream_properties_t *p_sp = &p_obj->stream_properties;
> -    size_t        i_peek;
> +    ssize_t i_peek;
>      const uint8_t *p_peek;
> 
> +#if UINT64_MAX > SSIZE_MAX
> +    if( p_sp->i_object_size > SSIZE_MAX )
> +    {
> +        msg_Err( s, "unable to peek: object size is larger than SSIZE_MAX"
> );

I wouldn´t name a constant in debug messages. Either format the value, or just 
write a generic too large.

> +        return VLC_EGENERIC;
> +    }
> +#endif
> +
>      if( ( i_peek = vlc_stream_Peek( s, &p_peek,  p_sp->i_object_size ) ) <
> 78 ) return VLC_EGENERIC;

(OT rant: I don´t like assignment as predicate where unnecessary.)

-- 
Rémi Denis-Courmont
http://www.remlab.net/