PR #24630 opened by iceray
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24630
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24630.patch
`c->low` holds a 32 bit CABAC register that is expected to wrap around, but
it is declared as a signed `int`, so the two renormalization shifts in
`cabac_functions.h` can shift the sign bit out of a signed value, which is
undefined behaviour. The C implementation of `get_cabac_inline()` is the one
used by sanitizer builds (`--disable-asm`), where UBSan aborts while decoding
a crafted H.264 stream:
```
libavcodec/cabac_functions.h:133:13: runtime error: left shift of 1090519039 by
1 places cannot be represented in type 'int'
#0 get_cabac_inline libavcodec/cabac_functions.h:133:13
#1 get_cabac_noinline libavcodec/cabac_functions.h:141:12
#2 ff_h264_decode_mb_cabac libavcodec/h264_cabac.c:2404:20
#3 decode_slice libavcodec/h264_slice.c:2631:23
#4 decode_nal_units libavcodec/h264dec.c:677:27
```
The same shift is also performed in `renorm_cabac_decoder_once()`, which is
reached through `get_cabac_terminate()`.
The patch performs both shifts in unsigned arithmetic and converts the result
back. The stored bit pattern is unchanged, so the signed comparisons and the
arithmetic right shift that the decoder performs on `c->low` keep operating on
exactly the same values.
Replayed with the OSS-Fuzz style decoder target (`tools/target_dec_h264_fuzzer`,
clang, ASan + UBSan):
| build | input | result |
|---|---|---|
| unpatched | 373-byte H.264 input | `cabac_functions.h:133:13` UBSan abort,
exit 1 |
| patched | same input | no diagnostic, exit 0 |
The input decodes as `error while decoding MB 0 4` on both builds, so the only
difference is the undefined shift. It is a raw H.264 bitstream; save the hex
below as `h264_cabac_signed_shift.poc` and run
`tools/target_dec_h264_fuzzer h264_cabac_signed_shift.poc`.
```
0000280000040000016742c01eda090100ff0167ff959595dca8db0167000001
68ff000000010165888400000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000004949494949494949494949494949494949494949
49324949497f26070090610000493d49494949494949494949494949ffffffff
ffffffffffffff49494949491600000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000
00f5c2fffe000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000
000000000000000000f5c2fffe9cff65ff301fff0021ec3cce800435005d0000
000010000000000000e5e5e5e5e5e503e5e5000130
```
>From 82f49440863f4b72fac03577573c14beaf735d41 Mon Sep 17 00:00:00 2001
From: iceray <[email protected]>
Date: Tue, 22 Sep 2026 05:57:39 +0000
Subject: [PATCH] avcodec/cabac_functions: shift the CABAC low register as
unsigned
c->low holds a 32 bit CABAC register that is expected to wrap around, but
it is declared as a signed int. Shifting the sign bit out of it is
undefined behaviour, and in builds that use the C implementation of
get_cabac_inline() (sanitizer builds configured with --disable-asm) UBSan
aborts while decoding a crafted H.264 stream:
libavcodec/cabac_functions.h:133:13: runtime error: left shift of
1090519039 by 1 places cannot be represented in type 'int'
#1 ff_h264_decode_mb_cabac libavcodec/h264_cabac.c:2404:20
The same shift is also performed in renorm_cabac_decoder_once(), which is
reached through get_cabac_terminate(). Perform both shifts in unsigned
arithmetic and convert the result back. The bit pattern is unchanged, so
the signed comparisons and the arithmetic right shift that the decoder
performs on c->low keep working on exactly the same values.
---
libavcodec/cabac_functions.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/cabac_functions.h b/libavcodec/cabac_functions.h
index 6a2e9ceca5..5c6f0022d3 100644
--- a/libavcodec/cabac_functions.h
+++ b/libavcodec/cabac_functions.h
@@ -79,7 +79,7 @@ static void refill(CABACContext *c){
static inline void renorm_cabac_decoder_once(CABACContext *c){
int shift= (uint32_t)(c->range - 0x100)>>31;
c->range<<= shift;
- c->low <<= shift;
+ c->low = (int)((unsigned)c->low << shift);
if(!(c->low & CABAC_MASK))
refill(c);
}
@@ -130,7 +130,7 @@ static av_always_inline int get_cabac_inline(CABACContext
*c, uint8_t * const st
lps_mask= ff_h264_norm_shift[c->range];
c->range<<= lps_mask;
- c->low <<= lps_mask;
+ c->low = (int)((unsigned)c->low << lps_mask);
if(!(c->low & CABAC_MASK))
refill2(c);
return bit;
--
2.52.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
