diff mbox series

[FFmpeg-devel,v4] lavc/vvc: Prevent overflow in chroma QP derivation

Message ID 20240609111824.39178-1-post@frankplowman.com
State New
Headers show
Series [FFmpeg-devel,v4] lavc/vvc: Prevent overflow in chroma QP derivation | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Frank Plowman June 9, 2024, 11:17 a.m. UTC 
On the top of p. 112 in VVC (09/2023):

It is a requirement of bitstream conformance that the values of
qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
of −QpBdOffset to 63, inclusive for i in the range of 0 to
numQpTables − 1, inclusive, and j in the range of 0 to
sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.

Additionally, don't discard the return code from sps_chroma_qp_table.

Signed-off-by: Frank Plowman <post@frankplowman.com>
---
Changes since v3:
* Add comment noting why qp_{in,out} are not tested themselves.

 libavcodec/vvc/ps.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Nuo Mi June 12, 2024, 12:55 p.m. UTC | #1 
On Sun, Jun 9, 2024 at 7:18 PM Frank Plowman <post@frankplowman.com> wrote:

> On the top of p. 112 in VVC (09/2023):
>
> It is a requirement of bitstream conformance that the values of
> qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
> of −QpBdOffset to 63, inclusive for i in the range of 0 to
> numQpTables − 1, inclusive, and j in the range of 0 to
> sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.
>
> Additionally, don't discard the return code from sps_chroma_qp_table.
>
> Applied.
Thank you, Frank
diff mbox series

Patch

diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 1b23675c98..92368eafc2 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -101,9 +101,14 @@  static int sps_chroma_qp_table(VVCSPS *sps)
 
         qp_out[0] = qp_in[0] = r->sps_qp_table_start_minus26[i] + 26;
         for (int j = 0; j < num_points_in_qp_table; j++ ) {
+            const uint8_t delta_qp_out = (r->sps_delta_qp_in_val_minus1[i][j] ^ r->sps_delta_qp_diff_val[i][j]);
             delta_qp_in[j] = r->sps_delta_qp_in_val_minus1[i][j] + 1;
+            // Note: we cannot check qp_{in,out}[j+1] here as qp_*[j] + delta_qp_*
+            //       may not fit in an 8-bit signed integer.
+            if (qp_in[j] + delta_qp_in[j] > 63 || qp_out[j] + delta_qp_out > 63)
+                return AVERROR(EINVAL);
             qp_in[j+1] = qp_in[j] + delta_qp_in[j];
-            qp_out[j+1] = qp_out[j] + (r->sps_delta_qp_in_val_minus1[i][j] ^ r->sps_delta_qp_diff_val[i][j]);
+            qp_out[j+1] = qp_out[j] + delta_qp_out;
         }
         sps->chroma_qp_table[i][qp_in[0] + off] = qp_out[0];
         for (int k = qp_in[0] - 1 + off; k >= 0; k--)
@@ -186,8 +191,11 @@  static int sps_derive(VVCSPS *sps, void *log_ctx)
     sps_inter(sps);
     sps_partition_constraints(sps);
     sps_ladf(sps);
-    if (r->sps_chroma_format_idc != 0)
-        sps_chroma_qp_table(sps);
+    if (r->sps_chroma_format_idc != 0) {
+        ret = sps_chroma_qp_table(sps);
+        if (ret < 0)
+            return ret;
+    }
 
     return 0;
 }