Skip to content

Clamp left_shift in oapv_dquant() to prevent shift UB - #294

Open
fkyslov wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
fkyslov:fix-dquant-left-shift-clamp
Open

fkyslov wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
fkyslov:fix-dquant-left-shift-clamp

Conversation

@fkyslov

@fkyslov fkyslov commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds defense-in-depth shift clamping in oapv_dquant() (src/oapv_tq.c):

  • Clamp left_shift = oapv_clip3(0, 30, -shift) before left-shifting coef[i] * q_matrix[i] in oapv_dquant().

Testing

  • Verified all 24 ctest unit/conformance tests pass with AddressSanitizer and UndefinedBehaviorSanitizer.

Clamp left_shift = oapv_clip3(0, 30, -shift) in oapv_dquant() as defense-in-depth against out-of-range shift exponents.

Signed-off-by: Fyodor Kyslov <kyslov@google.com>
Comment thread src/oapv_tq.c
}
else {
int left_shift = -shift;
int left_shift = oapv_clip3(0, 30, -shift);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clamp is not needed. left_shift is already bounded by the bitstream validation:

  • dq_shift is derived as bit_depth - 2 - qp / 6.
  • bit_depth is checked to be in [10, 16], and tile_qp in [0, MAX_QUANT(bit_depth)], where MAX_QUANT(bit_depth) = 63 + (bit_depth - 10) * 6.
  • So qp / 6 is at most bit_depth, and dq_shift is always in [-2, bit_depth - 2].

In this branch left_shift is therefore always in [0, 2], and oapv_clip3(0, 30, -shift) can never change it. It would only add an operation to the dequantization path, so this change cannot be accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants