[FFmpeg-devel] [PATCH] golomb: always check for invalid UE golomb codes in get_ue_golomb

[Michael Niedermayer]

On Mon, Dec 14, 2015 at 08:58:46PM +0100, Andreas Cadhalpun wrote:
> On 14.12.2015 00:51, Michael Niedermayer wrote:
> > On Sun, Dec 13, 2015 at 09:56:06PM +0100, Andreas Cadhalpun wrote:
> >> Also correct the check to reject log < 7, because UPDATE_CACHE only
> >> guarantees 25 meaningful bits.
> >>
> >> This fixes undefined behavior:
> >> runtime error: shift exponent is negative
> >>
> >> Testing with START/STOP timers in get_ue_golomb, one for the first
> >> branch (A) and one for the second (B), shows that there is practically no
> >> slowdown, e.g. for the cavs decoder:
> >>
> >> With the check in the B branch:
> >>     629 decicycles in get_ue_golomb B, 4194260 runs,     44 skips
> >>     433 decicycles in get_ue_golomb A,268434102 runs,   1354 skips
> >>
> >> Without the check:
> >>     624 decicycles in get_ue_golomb B, 4194273 runs,     31 skips
> >>     433 decicycles in get_ue_golomb A,268434203 runs,   1253 skips
> >>
> >> Since the B branch is executed far less often than the A branch, this
> >> change is negligible, even more so for the h264 decoder, where the ratio
> >> B/A is a lot smaller.
> >>
> >> Fixes: mozilla bug 1229208
> >> Fixes: fbeb8b2c7c996e9b91c6b1af319d7ebc/asan_heap-oob_195450f_2743_e8856ece4579ea486670be2b236099a0.bit
> >>
> >> Found-by: Tyson Smith
> >> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>
> >> Note that I just copied the "Fixes:" lines from Michael's patch, but actually
> >> I don't know what mozilla bug 1229208 is about, as it seems not to be public.
> >> Also I don't have the mentioned sample, but the patch fixes more than 1000
> >> of my fuzzed samples that triggered this ubsan error, so I'm confident the
> >> mentioned one is also fixed.
> > 
> > actually i think the bug number is
> > "Bug 1230239 - FFMPEG: shift exponent is negative in [@get_ue_golomb] "
> 
> I changed the bug number accordingly,
> 
> > patch should be ok
> 
> and pushed the patch.
> 
> > and iam also not happy about the bugs being non public
> > i tried unchecking "Security-Sensitive Media Bug" but i seem not to
> > have the power to do that but its quite possibly iam doing something
> > wrong
> 
> Maybe add a comment requesting the bug to be made public, so that
> someone who has that power can do it.

i think i suggested already in a few of the bugs that they are
probably not security relevant

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151214/aa88b112/attachment.sig>