Hi FFmpeg devs,
An audit of the VVC SPS chroma QP table path in 8.1.2 / current master
turned up an off-by-one in the sizing of VVC_MAX_POINTS_IN_QP_TABLE.
It is NOT currently reachable through the decoder, so this is a
hardening fix rather than a security report. Patch is included below.
======================================================================
1. Defect
======================================================================
libavcodec/vvc/vvc.h:121:
//so sps_num_points_in_qp_table_minus1[i] should in range [0, 62 +
QpBdOffset]
//since 16 bits QpBdOffset is 48,
sps_num_points_in_qp_table_minus1[i] should range [0, 110]
VVC_MAX_POINTS_IN_QP_TABLE = 111,
The range derivation is correct; the sizing is not. The constant is
sized to the upper bound of num_points_in_qp_table_minus1 (110 at
16-bit, QpBdOffset=48), giving 111 -- i.e. it holds "the number of
points". But the loops in sps_chroma_qp_table() (ps.c) write *index*
num_points_in_qp_table (worst case 111), not num_points - 1. An array
accessed at index N must hold N+1 slots (0..N); here it holds N, so it
is one short.
Two OOB write sites in sps_chroma_qp_table() (ps.c), into arrays of
size VVC_MAX_POINTS_IN_QP_TABLE (valid indices 0..110):
(a) Stack OOB write, ps.c:113-114
int8_t qp_in[VVC_MAX_POINTS_IN_QP_TABLE], qp_out[...];
for (int j = 0; j < num_points_in_qp_table; j++) { // j in [0, 110]
...
qp_in [j+1] = qp_in [j] + delta_qp_in[j]; //
j=110 -> qp_in [111] OOB
qp_out[j+1] = qp_out[j] + delta_qp_out; //
j=110 -> qp_out[111] OOB
}
Each write is 1 byte; the value is bounded by the `> 63` guard
at ps.c:111 (added in d72a5fe), which caps qp_in[j+1]/qp_out[j+1]
<= 63.
(b) Heap OOB write, ps.c:122-124
int off = sps->qp_bd_offset; // 48 for 16-bit
for (int j = 0; j < num_points_in_qp_table; j++)
for (int k = qp_in[j]+1+off; k <= qp_in[j+1]+off; k++)
sps->chroma_qp_table[i][k] = ...; //
k_max = 63+48 = 111 OOB
chroma_qp_table is the trailing field of VVCSPS (ps.h:84),
so index 111 lands past the end of the allocation.
CBS admits the triggering range:
cbs_h266_syntax_template.c:1412 ses(sps_qp_table_start_minus26[i],
-26 - qp_bd_offset, 36, ...)
cbs_h266_syntax_template.c:1413 ues(sps_num_points_in_qp_table_minus1[i],
0, 36 - sps_qp_table_start_minus26[i], ...)
// start=-74, qpb=48 -> upper bound 110
Correct size is 112 (indices 0..111).
======================================================================
2. Reachability (why it is dead today)
======================================================================
Both writes require index 111. Index 111 requires QpBdOffset = 48:
- (a) num_points = 111 => num_points_minus1 = 110
CBS bound: num_points_minus1 <= 36 - start_minus26
=> start_minus26 <= -74
CBS lower bound: start_minus26 >= -26 - QpBdOffset
=> QpBdOffset >= 48
- (b) k_max = qp_in_max + off = 63 + off = 111 => off = 48
QpBdOffset = 6 * (BitDepth - 8) => QpBdOffset = 48 <=> BitDepth = 16
(sps_bitdepth_minus8 = 8, admitted by CBS at line 1279).
But sps_map_pixel_format() (ps.c:35) accepts only {8,10,12} and returns
AVERROR_INVALIDDATA for anything else. It is reached via sps_bit_depth()
at ps.c:236 -- inside sps_derive(), *before* the sps_chroma_qp_table()
call at ps.c:267:
sps_derive (ps.c:231):
ret = sps_bit_depth(sps, c); // ps.c:236 ->
sps_map_pixel_format
if (ret < 0) return ret; // 16-bit aborts here
...
if (r->sps_chroma_format_idc != 0)
ret = sps_chroma_qp_table(sps); // ps.c:267 --
never reached for 16-bit
At the deepest supported depth (12-bit, QpBdOffset = 24) the max index
is 63 + 24 = 87 < 111. No path through the decoder can index 111.
CBS does not call sps_chroma_qp_table() (decoder-side only), so the CBS
read/write path is not an alternative entry.
The trigger condition (BitDepth=16) and the decoder's acceptance
condition (BitDepth in {8,10,12}) are mutually exclusive -- the path is
deadlocked.
======================================================================
3. Verification
======================================================================
Built 8.1.2 with --disable-asm --disable-optimizations --enable-debug=3
and -fsanitize=address -fno-omit-frame-pointer -O0 (gcc 15, aarch64).
Drove the SPS decode path directly by exposing sps_alloc() (normally
file-static) and constructing an H266RawSPS via av_refstruct_allocz
with the extreme fields:
r->sps_bitdepth_minus8 = 8;
r->sps_chroma_format_idc = 2;
r->sps_same_qp_table_for_chroma_flag = 0;
r->sps_joint_cbcr_enabled_flag = 1; // num_qp_tables = 3
r->sps_qp_table_start_minus26[i] = -74; // i = 0..2
r->sps_num_points_in_qp_table_minus1[i] = 110;
r->sps_delta_qp_in_val_minus1[i][j] = 0; // delta_qp_in = 1
r->sps_delta_qp_diff_val[i][j] = 0;
bitdepth=16: rejected by sps_map_pixel_format, sps_alloc returns NULL,
no ASan report (function body not entered).
bitdepth=12: max index 87, no ASan report.
bitdepth=10: max index 75, no ASan report.
Consistent with the reachability argument above.
======================================================================
4. Fix
======================================================================
Bump VVC_MAX_POINTS_IN_QP_TABLE 111 -> 112. All arrays driven by it
(qp_in, qp_out, delta_qp_in, chroma_qp_table, and the CBS
sps_delta_qp_* tables in cbs_h266.h:399-400) resize consistently and
become correctly sized for the full CBS-admitted range.
Alternative: an explicit bounds check inside sps_chroma_qp_table()
(num_points_in_qp_table < VVC_MAX_POINTS_IN_QP_TABLE) would also work,
but the constant is the root cause and this is the smaller, more
complete change.
References: d72a5fe ("lavc/vvc: Prevent overflow in chroma QP derivation"),
a357758 ("vvc.h: Enable 16-bit support for
VVC_MAX_POINTS_IN_QP_TABLE")
-- >8 --
Subject: [PATCH] lavc/vvc: fix off-by-one in sps_chroma_qp_table sizing
VVC_MAX_POINTS_IN_QP_TABLE is sized for num_points_in_qp_table_minus1's
upper bound (110 at 16-bit, QpBdOffset=48), giving 111. But
sps_chroma_qp_table() writes index num_points_in_qp_table (== 111 in the
worst case) into qp_in/qp_out, and chroma_qp_table[i][63 + QpBdOffset]
(== chroma_qp_table[i][111]) -- both off-by-one on arrays of size 111.
Currently unreachable: sps_map_pixel_format() rejects 16-bit before
sps_chroma_qp_table() runs, and at the deepest supported depth (12-bit)
the max index is 63 + 24 = 87. Bump the constant to 112 as hardening so
that adding 16-bit support does not turn this into a live OOB write.
Signed-off-by: smile1110 <[email protected]>
---
libavcodec/vvc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vvc.h b/libavcodec/vvc.h
--- a/libavcodec/vvc.h
+++ b/libavcodec/vvc.h
@@ -118,7 +118,7 @@
//so sps_num_points_in_qp_table_minus1[i] should in range [0, 62
+ QpBdOffset]
//since 16 bits QpBdOffset is 48,
sps_num_points_in_qp_table_minus1[i] should range [0, 110]
- VVC_MAX_POINTS_IN_QP_TABLE = 111,
+ VVC_MAX_POINTS_IN_QP_TABLE = 112,
// 7.4.6.1: hrd_cpb_cnt_minus1 is in [0, 31].
--
Sincerely
smile1110
China
To: [email protected]
Subject: [PATCH] lavc/vvc: fix off-by-one in sps_chroma_qp_table sizing
Content-Type: text/plain; charset=UTF-8
Hi FFmpeg devs,
An audit of the VVC SPS chroma QP table path in 8.1.2 / current master
turned up an off-by-one in the sizing of VVC_MAX_POINTS_IN_QP_TABLE.
It is NOT currently reachable through the decoder, so this is a
hardening fix rather than a security report. Patch is included below.
======================================================================
1. Defect
======================================================================
libavcodec/vvc/vvc.h:121:
//so sps_num_points_in_qp_table_minus1[i] should in range [0, 62 + QpBdOffset]
//since 16 bits QpBdOffset is 48, sps_num_points_in_qp_table_minus1[i] should range [0, 110]
VVC_MAX_POINTS_IN_QP_TABLE = 111,
The range derivation is correct; the sizing is not. The constant is
sized to the upper bound of num_points_in_qp_table_minus1 (110 at
16-bit, QpBdOffset=48), giving 111 -- i.e. it holds "the number of
points". But the loops in sps_chroma_qp_table() (ps.c) write *index*
num_points_in_qp_table (worst case 111), not num_points - 1. An array
accessed at index N must hold N+1 slots (0..N); here it holds N, so it
is one short.
Two OOB write sites in sps_chroma_qp_table() (ps.c), into arrays of
size VVC_MAX_POINTS_IN_QP_TABLE (valid indices 0..110):
(a) Stack OOB write, ps.c:113-114
int8_t qp_in[VVC_MAX_POINTS_IN_QP_TABLE], qp_out[...];
for (int j = 0; j < num_points_in_qp_table; j++) { // j in [0, 110]
...
qp_in [j+1] = qp_in [j] + delta_qp_in[j]; // j=110 -> qp_in [111] OOB
qp_out[j+1] = qp_out[j] + delta_qp_out; // j=110 -> qp_out[111] OOB
}
Each write is 1 byte; the value is bounded by the `> 63` guard
at ps.c:111 (added in d72a5fe), which caps qp_in[j+1]/qp_out[j+1]
<= 63.
(b) Heap OOB write, ps.c:122-124
int off = sps->qp_bd_offset; // 48 for 16-bit
for (int j = 0; j < num_points_in_qp_table; j++)
for (int k = qp_in[j]+1+off; k <= qp_in[j+1]+off; k++)
sps->chroma_qp_table[i][k] = ...; // k_max = 63+48 = 111 OOB
chroma_qp_table is the trailing field of VVCSPS (ps.h:84),
so index 111 lands past the end of the allocation.
CBS admits the triggering range:
cbs_h266_syntax_template.c:1412 ses(sps_qp_table_start_minus26[i], -26 - qp_bd_offset, 36, ...)
cbs_h266_syntax_template.c:1413 ues(sps_num_points_in_qp_table_minus1[i],
0, 36 - sps_qp_table_start_minus26[i], ...)
// start=-74, qpb=48 -> upper bound 110
Correct size is 112 (indices 0..111).
======================================================================
2. Reachability (why it is dead today)
======================================================================
Both writes require index 111. Index 111 requires QpBdOffset = 48:
- (a) num_points = 111 => num_points_minus1 = 110
CBS bound: num_points_minus1 <= 36 - start_minus26
=> start_minus26 <= -74
CBS lower bound: start_minus26 >= -26 - QpBdOffset
=> QpBdOffset >= 48
- (b) k_max = qp_in_max + off = 63 + off = 111 => off = 48
QpBdOffset = 6 * (BitDepth - 8) => QpBdOffset = 48 <=> BitDepth = 16
(sps_bitdepth_minus8 = 8, admitted by CBS at line 1279).
But sps_map_pixel_format() (ps.c:35) accepts only {8,10,12} and returns
AVERROR_INVALIDDATA for anything else. It is reached via sps_bit_depth()
at ps.c:236 -- inside sps_derive(), *before* the sps_chroma_qp_table()
call at ps.c:267:
sps_derive (ps.c:231):
ret = sps_bit_depth(sps, c); // ps.c:236 -> sps_map_pixel_format
if (ret < 0) return ret; // 16-bit aborts here
...
if (r->sps_chroma_format_idc != 0)
ret = sps_chroma_qp_table(sps); // ps.c:267 -- never reached for 16-bit
At the deepest supported depth (12-bit, QpBdOffset = 24) the max index
is 63 + 24 = 87 < 111. No path through the decoder can index 111.
CBS does not call sps_chroma_qp_table() (decoder-side only), so the CBS
read/write path is not an alternative entry.
The trigger condition (BitDepth=16) and the decoder's acceptance
condition (BitDepth in {8,10,12}) are mutually exclusive -- the path is
deadlocked.
======================================================================
3. Verification
======================================================================
Built 8.1.2 with --disable-asm --disable-optimizations --enable-debug=3
and -fsanitize=address -fno-omit-frame-pointer -O0 (gcc 15, aarch64).
Drove the SPS decode path directly by exposing sps_alloc() (normally
file-static) and constructing an H266RawSPS via av_refstruct_allocz
with the extreme fields:
r->sps_bitdepth_minus8 = 8;
r->sps_chroma_format_idc = 2;
r->sps_same_qp_table_for_chroma_flag = 0;
r->sps_joint_cbcr_enabled_flag = 1; // num_qp_tables = 3
r->sps_qp_table_start_minus26[i] = -74; // i = 0..2
r->sps_num_points_in_qp_table_minus1[i] = 110;
r->sps_delta_qp_in_val_minus1[i][j] = 0; // delta_qp_in = 1
r->sps_delta_qp_diff_val[i][j] = 0;
bitdepth=16: rejected by sps_map_pixel_format, sps_alloc returns NULL,
no ASan report (function body not entered).
bitdepth=12: max index 87, no ASan report.
bitdepth=10: max index 75, no ASan report.
Consistent with the reachability argument above.
======================================================================
4. Fix
======================================================================
Bump VVC_MAX_POINTS_IN_QP_TABLE 111 -> 112. All arrays driven by it
(qp_in, qp_out, delta_qp_in, chroma_qp_table, and the CBS
sps_delta_qp_* tables in cbs_h266.h:399-400) resize consistently and
become correctly sized for the full CBS-admitted range.
Alternative: an explicit bounds check inside sps_chroma_qp_table()
(num_points_in_qp_table < VVC_MAX_POINTS_IN_QP_TABLE) would also work,
but the constant is the root cause and this is the smaller, more
complete change.
References: d72a5fe ("lavc/vvc: Prevent overflow in chroma QP derivation"),
a357758 ("vvc.h: Enable 16-bit support for VVC_MAX_POINTS_IN_QP_TABLE")
-- >8 --
Subject: [PATCH] lavc/vvc: fix off-by-one in sps_chroma_qp_table sizing
VVC_MAX_POINTS_IN_QP_TABLE is sized for num_points_in_qp_table_minus1's
upper bound (110 at 16-bit, QpBdOffset=48), giving 111. But
sps_chroma_qp_table() writes index num_points_in_qp_table (== 111 in the
worst case) into qp_in/qp_out, and chroma_qp_table[i][63 + QpBdOffset]
(== chroma_qp_table[i][111]) -- both off-by-one on arrays of size 111.
Currently unreachable: sps_map_pixel_format() rejects 16-bit before
sps_chroma_qp_table() runs, and at the deepest supported depth (12-bit)
the max index is 63 + 24 = 87. Bump the constant to 112 as hardening so
that adding 16-bit support does not turn this into a live OOB write.
Signed-off-by: smile1110 <[email protected]>
---
libavcodec/vvc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vvc.h b/libavcodec/vvc.h
--- a/libavcodec/vvc.h
+++ b/libavcodec/vvc.h
@@ -118,7 +118,7 @@
//so sps_num_points_in_qp_table_minus1[i] should in range [0, 62 + QpBdOffset]
//since 16 bits QpBdOffset is 48, sps_num_points_in_qp_table_minus1[i] should range [0, 110]
- VVC_MAX_POINTS_IN_QP_TABLE = 111,
+ VVC_MAX_POINTS_IN_QP_TABLE = 112,
// 7.4.6.1: hrd_cpb_cnt_minus1 is in [0, 31].
--
Sincerely
smile1110
China
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
