From 590d8f64399ae3771861155422f22664f078c9c9 Mon Sep 17 00:00:00 2001
From: Yixuan Tang <114117134+plain-noodle-expert@users.noreply.github.com>
Date: Thu, 11 Jun 2026 18:08:05 +0800
Subject: [PATCH] Merge pull request #29240 from
 plain-noodle-expert:fix/brisk-ubsan-negative-shift
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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`
---
 modules/features2d/src/brisk.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules/features2d/src/brisk.cpp b/modules/features2d/src/brisk.cpp
index 4be003e1cd..7eaf5880cf 100644
--- a/modules/features2d/src/brisk.cpp
+++ b/modules/features2d/src/brisk.cpp
@@ -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;