[vlc-devel] [PATCH] demux/aiff: fix integer overflow leading to infinite loop

[Rémi Denis-Courmont]

Le lauantaina 29. lokakuuta 2016, 0.10.41 EEST Filip Roséen a écrit :
> The previous implementation could potentially overflow the uint32_t
> holding the data-size when adding the size of the mandatory header, in
> order to consume the entire chunk. If that happened we could end up in
> an infinite loop (given that we are not guaranteed to make progress).
> 
> These changes fixes the issue by introducing another variable that is
> only used for the purpose of storing the chunk (total) size.
> 
> fixes #17562
> 
> --
> 
> I will clean-up the implementation shortly, but thought I would
> address this immidiate issue as soon as possible.
> 
> The demuxer can be made a lot simpler, and a lot safer.
> ---
>  modules/demux/aiff.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/modules/demux/aiff.c b/modules/demux/aiff.c
> index bdd0308..02cc711 100644
> --- a/modules/demux/aiff.c
> +++ b/modules/demux/aiff.c
> @@ -123,14 +123,13 @@ static int Open( vlc_object_t *p_this )
> 
>      for( ;; )
>      {
> -        uint32_t i_size;
> -
>          if( vlc_stream_Peek( p_demux->s, &p_peek, 8 ) < 8 )
>              goto error;
> 
> -        i_size = GetDWBE( &p_peek[4] );
> +        uint32_t i_data_size = GetDWBE( &p_peek[4] );
> +        ssize_t i_chunk_size = ( i_data_size + 8 + 1 ) ^ 1;

On 32-bits, this is undefined overflow if i_data_size > 0x7ffffff6.

-- 
Rémi Denis-Courmont
Nonsponsored VLC developer
http://www.remlab.net/CV.pdf