Skip to content

xfeatures2d: fix DAISY roi descriptor buffer overflow - #4175

Open
shimat wants to merge 1 commit into
opencv:5.xfrom
shimat:fix/daisy-roi-buffer-overflow
Open

xfeatures2d: fix DAISY roi descriptor buffer overflow#4175
shimat wants to merge 1 commit into
opencv:5.xfrom
shimat:fix/daisy-roi-buffer-overflow

Conversation

@shimat

@shimat shimat commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

ComputeDescriptorsInvoker computed the output row index as an absolute image offset (y*image->cols + x), but the descriptors buffer passed to DAISY::compute(image, roi, descriptors) is only roi.width*roi.height rows, addressed relative to the roi. Unless roi covers the whole image, this writes past the end of the allocated buffer.

Store the roi's y offset alongside the existing x offset and compute the row index relative to the roi. The absolute (y, x) passed to get_unnormalized_descriptor() is unchanged, since sampling is done against the full-image gradient layers regardless of roi.

Added a regression test in test_daisy.cpp comparing descriptors computed via a roi against the same pixels computed via the full image.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

ComputeDescriptorsInvoker computed the output row index as an absolute
image offset (y*image->cols + x), but the descriptors buffer passed to
DAISY::compute(image, roi, descriptors) is only roi.width*roi.height
rows, addressed relative to the roi. Unless roi covers the whole image
(x == 0, y == 0, width == image.cols), this writes past the end of the
allocated buffer.

Store the roi's y offset alongside the existing x offset and compute
the row index relative to the roi. The absolute (y, x) passed to
get_unnormalized_descriptor() is unchanged, since sampling is done
against the full-image gradient layers regardless of roi.

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI disclosure: I used OpenAI Codex to assist this review; I verified the result locally against exact head 47e345fb70f4f515195423605ad158654ce2416f.

Looks good. The ROI-sized descriptor matrix now uses ROI-relative row indices while descriptor sampling remains in absolute image coordinates. I built the exact head against current OpenCV 5.x core and the new ROI/full-image equivalence regression passes.

As a deterministic control, I rolled back only the three production hunks while retaining the submitted test. The same test then fails across the ROI (maximum descriptor differences around 0.06 to 0.11); restoring the exact-head implementation makes it pass again. git diff --check is clean.

The retained upstream reds do not implicate this change: the Windows job exits before checkout because its support script is missing, while Ubuntu 26.04 fails unrelated GStreamer/G-API and BRISQUE tests. The other ten reported checks are green.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants