flac dev - Jan 2016 - [PATCH] add a check in FLAC__bitwriter_write_raw_uint32()

FLAC__bitwriter_write_raw_uint32() assumes that the unused bits of the
'val' argument are equal to zero, and doesn't check it (an assert
was
added recently, but it's disabled in non-debug builds anyway).

But it makes more difficult to catch metadata length overflow (such as too
big picture or padding size), so there's a reason to add such check.


The function FLAC__bitwriter_write_raw_uint32() simply check sthat unused bits
are 0

     if((bits < 32) && (val>>bits != 0))
         return false;

and then calls FLAC__bitwriter_write_raw_uint32_nocheck().

The new function FLAC__bitwriter_write_raw_uint32_nocheck() is
an old FLAC__bitwriter_write_raw_uint32() that doesn't do this check,
and all other functions in bitwriter.c call it if they can guarantee
that all unused bits are 0.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: write_bits_check.zip
Type: application/zip
Size: 1793 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20160108/5d37e462/attachment.zip

lvqcl wrote:
> FLAC__bitwriter_write_raw_uint32() assumes that the unused bits of the
> 'val' argument are equal to zero, and doesn't check it (an
assert was
> added recently, but it's disabled in non-debug builds anyway).
> 
> But it makes more difficult to catch metadata length overflow (such as too
> big picture or padding size), so there's a reason to add such check.
> 
> 
> The function FLAC__bitwriter_write_raw_uint32() simply check sthat unused
bits are 0
> 
>      if((bits < 32) && (val>>bits != 0))
>          return false;
> 
> and then calls FLAC__bitwriter_write_raw_uint32_nocheck().
> 
> The new function FLAC__bitwriter_write_raw_uint32_nocheck() is
> an old FLAC__bitwriter_write_raw_uint32() that doesn't do this check,
> and all other functions in bitwriter.c call it if they can guarantee
> that all unused bits are 0.
Applied. Thanks.

Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/