Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fkyslov wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/oapv_tq.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ static void oapv_dquant(s16 *coef, s16 q_matrix[OAPV_BLK_D], int log2_w, int log
}
}
else {
int left_shift = -shift;
int left_shift = oapv_clip3(0, 30, -shift);

@kpchoi kpchoi Sep 28, 2026

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.

for(i = 0; i < pixels; i++) {
lev = (coef[i] * q_matrix[i]) << left_shift;
coef[i] = (s16)oapv_clip3(-32768, 32767, lev);
Expand Down
Loading