Fix UB in CopyMakeConstBoder_8u#19603
Conversation
|
Could you elaborate on |
modules/core/src/copy.cpp
Outdated
| for( i = 0; i < top; i++ ) | ||
| memcpy(dst + (i - top)*dststep, constBuf, dstroi.width); | ||
| memcpy(dst + (i - top)*static_cast<int>(dststep), constBuf, dstroi.width); | ||
|
|
||
| for( i = 0; i < bottom; i++ ) | ||
| memcpy(dst + (i + srcroi.height)*dststep, constBuf, dstroi.width); |
There was a problem hiding this comment.
Please add problem description.
Are you sure that this patch fixes your problem? Because there is another similar code few lines below.
There was a problem hiding this comment.
I added a more detailed description of the problem I found.
I am sure it fixed the UB I found. This is the only part I am using so far in my code so I cannot tell whether there are/aren't more UB similar to this one, but at least this one is fixed. I will be more than happy to fix other as I cross them my way, though.
There was a problem hiding this comment.
I added a more detailed description of the problem.
I am sure it fixed the UB I found. This is the only part I am using so far in my code so I cannot tell whether there are/aren't more UB similar to this one, but at least this one is fixed. I will be more than happy to fix other as I cross them my way, though.
There was a problem hiding this comment.
The fix will fail if dststep is greater than INT_MAX. That's not the most often case, but possible.
Also i-top is always negative so the fix could be dst - (top - i)*dststep
Caused by overflow of arithmetic operators conversion rank
|
Patch is rebased on 3.4 branch and updated to avoid using of pointers arithmetic with negative offsets. @federicohml Please take a look (we do no have reproducer on our side). |
Problem: addition of unsigned offset to 0x000007ff0362 overflowed to 0x000007fefac0
In
third_party/opencv/modules/core/src/copy.cppthe functioncopyMakeConstBorder_8ureceives the step of a cv::Mat, namelydststepof typesize_t. When offsetting the destination matrix pointer to start copying data,memcpy(dst + (i - top) * dststep, constBuf, dstroi.width)the result of(i - top)could be negative, then, when applying the arithmetic operator * because of arithmetic operator conversion the int is converted to size_t, resulting pointer overflow.Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.