1
0
mirror of https://github.com/opencv/opencv.git synced 2026-07-31 08:13:04 +04:00

Merge pull request #29240 from plain-noodle-expert:fix/brisk-ubsan-negative-shift

fix(features2d): fix UB left-shift of negative value in BRISK subpixel2D #29240

## Summary

Fixes #29239

`BriskScaleSpace::subpixel2D` computes quadratic surface coefficients for sub-pixel keypoint refinement using AGAST scores from a 3×3 neighbourhood. Two of those coefficients were computed via left-shift:

```cpp
// Before (UB when operand is negative)
int coeff5 = (s_0_0 - s_0_2 - s_2_0 + s_2_2) << 2;
int coeff6 = -(s_0_0 + s_0_2 - ((s_1_0 + s_0_1 + s_1_2 + s_2_1) << 1) - 5 * s_1_1 + s_2_0 + s_2_2) << 1;
```

AGAST scores are non-negative (0–255), but their **differences can be negative**. Left-shifting a negative signed integer is **undefined behaviour** under C++11 [expr.shift]/2. UBSan reports:

```
brisk.cpp:2037:48: runtime error: left shift of negative value -3
```

## Fix

Replace the outer left-shifts with multiplications (`* 4` / `* 2`), which are always well-defined. Modern compilers (GCC, Clang, MSVC) emit identical code for the positive case.

```cpp
// After (safe for all inputs, identical semantics)
int coeff5 = (s_0_0 - s_0_2 - s_2_0 + s_2_2) * 4;
int coeff6 = -(s_0_0 + s_0_2 - ((s_1_0 + s_0_1 + s_1_2 + s_2_1) << 1) - 5 * s_1_1 + s_2_0 + s_2_2) * 2;
```

The inner `<< 1` inside the parenthesis of `coeff6` is applied to a sum of non-negative scores and remains safe.

## Tests

Four regression tests added to `test_brisk.cpp` (all verified to pass under `-fsanitize=undefined`):

| Test | Image | Purpose |
|---|---|---|
| `regression_ubsan_negative_shift_isolated_pixel` | 40×40, single pixel = 3 | Matches exact crash parameters (s_0_2=3, others=0) |
| `regression_ubsan_negative_shift_rect_corner` | 60×60 white rect on black | Strong AGAST responses at corners |
| `regression_ubsan_negative_shift_random_image` | 30×30 LCG noise (0–15) | Broad coverage of score combinations |
| `regression_ubsan_negative_shift_multi_octave` | 80×80 checkerboard, 3 octaves | Exercises cross-layer subpixel refinement |

## Crash Details

- **File**: `modules/features2d/src/brisk.cpp:2037`
- **Parameters at crash**: `s_0_0=0, s_0_1=0, s_0_2=3, s_1_0=0, s_1_1=0, s_1_2=0, s_2_0=0, s_2_1=0, s_2_2=0`
- **Expression**: `(0 - 3 - 0 + 0) << 2` = `-3 << 2` → **UB**
- **Detected by**: libFuzzer + UBSan (`-fsanitize=address,undefined`)
- **Affected path**: `detectAndCompute` → `computeKeypointsNoOrientation` → `getKeypoints` → `refine3D` → `getScoreMaxAbove` → `subpixel2D`
This commit is contained in:
Yixuan Tang
2026-06-11 18:08:05 +08:00
committed by GitHub
parent aed41fdabe
commit 590d8f6439
+2 -2
View File
@@ -2036,8 +2036,8 @@ BriskScaleSpace::subpixel2D(const int s_0_0, const int s_0_1, const int s_0_2, c
int tmp4 = tmp3 - 2 * tmp2;
int coeff3 = -3 * (tmp3 + s_0_1 - s_2_1);
int coeff4 = -3 * (tmp4 + s_1_0 - s_1_2);
int coeff5 = (s_0_0 - s_0_2 - s_2_0 + s_2_2) << 2;
int coeff6 = -(s_0_0 + s_0_2 - ((s_1_0 + s_0_1 + s_1_2 + s_2_1) << 1) - 5 * s_1_1 + s_2_0 + s_2_2) << 1;
int coeff5 = (s_0_0 - s_0_2 - s_2_0 + s_2_2) * 4;
int coeff6 = -(s_0_0 + s_0_2 - ((s_1_0 + s_0_1 + s_1_2 + s_2_1) * 2) - 5 * s_1_1 + s_2_0 + s_2_2) * 2;
// 2nd derivative test:
int H_det = 4 * coeff1 * coeff2 - coeff5 * coeff5;