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

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Sun Dec 13 21:56:06 CET 2015


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.

---
 libavcodec/golomb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index d30bb6b..5136a04 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
         int log = 2 * av_log2(buf) - 31;
         LAST_SKIP_BITS(re, gb, 32 - log);
         CLOSE_READER(re, gb);
-        if (CONFIG_FTRAPV && log < 0) {
+        if (log < 7) {
             av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
             return AVERROR_INVALIDDATA;
         }
-- 
2.6.2


More information about the ffmpeg-devel mailing list