ORB out-out-bounds memory access in 2.4.2 (Bug #2216)
| Status: | Done | Start date: | 2012-07-31 |
|---|---|---|---|
| Priority: | Normal | Due date: | |
| Assignee: |  Andrey Kamaev |
% Done: | 0% |
| Category: | features2d | ||
| Target version: | 2.4.3 | ||
| Affected version: | Operating System: | ||
| Difficulty: | HW Platform: | ||
| Pull request: |
Description
Please change:
modules/features2d/src/orb.cpp:636 vector<int> umax(halfPatchSize + 1);
to:
modules/features2d/src/orb.cpp:636 vector<int> umax(halfPatchSize + 2);
Otherwise vmax will get out of bound on the last write. I don't know why it is so, hopefully someone will offer a better fix.
Found with Valgrind.
In addition, this fixes heap corruption in Debug MSVC++ 2010 where a canary gets modified.
If you need a sample program, I can provide address to my github repo.
Related issues
| related to Bug #2271: Algorithm parameter setters don't validate the parameters | Cancelled | 2012-08-15 |
Associated revisions
#2216 Forbid incorrect patchSize values in ORB
Merge pull request #2216 from ElenaGvozdeva:ocl_MOG2
History
Updated by Daniil Osokin over 12 years ago
Sure, please provide source code to reproduce out of bound access and right behaviour.
Updated by Stanislaw Halik over 12 years ago
While the source code is available, it's a complex program that reaches a RANSAC consensus while tracking features: https://github.com/sthalik/headtracker
Anyway, here's a 'proof':
patch-size of 1:
int halfPatchSize = 1 / 2; // 0
vector<int> umax(halfPatchSize + 1); // 1
int vmax = cvFloor(halfPatchSize * sqrt(2.f) / 2 + 1); // 1
Here <= is used for loop termination clause. So either patch-size below 2 is considered an incorrect parameter, and should be caught in the detector's constructor, or it is correct, and the buffer is too small. Overrunning it on a bad param is nasty.
Updated by Daniil Osokin over 12 years ago
Thank you, we will process it.
Updated by Andrey Kamaev over 12 years ago
Thanks for the report.
Fix blocking invalid patch sizes is pushed into master.
- Status changed from Open to Done
- Target version set to 2.4.3
- Assignee changed from Maria Dimashova to Andrey Kamaev