Add check: invalidPointerOverlapTest for unsafe pointer overflow comparison#8620
Add check: invalidPointerOverlapTest for unsafe pointer overflow comparison#8620tmleman wants to merge 1 commit into
Conversation
tmleman
force-pushed
the
topic/checkcondition/new/pointer_overflow
branch
from
fcbffc1 to
8d9b4ba
Compare
…arison
Add a new inconclusive warning that detects the pointer overlap idiom
p1 >= p2 && p1 < p2 + n (or its mirror / cast variants)
where p1 and p2 are distinct pointers and n is an unsigned offset, e.g.
the memcpy_s overlap check '(dest >= src && dest < src + count) || ...'.
The 'p1 < p2 + n' clause assumes 'p2 + n' does not overflow past the end
of the object. If n is large enough that 'p2 + n' wraps (UB), some
mainstream compilers assume the wrap cannot happen and fold the
comparison, silently dropping the check. The remedy in user code is to
cast the pointers to uintptr_t and compare in integer space, with an
explicit overflow guard.
To avoid false positives, the warning only fires when the comparison is
paired (via &&) with another comparison of the same two pointers, which
identifies the overlap idiom; plain bounds checks like 'p < buf + len'
and room checks like 'cur + need <= limit' are not flagged. The check is
gated behind --enable=inconclusive. The same-pointer case 'p < p + n' is
left to the existing invalidTestForOverflow check.
This pattern was found in a real memcpy_s overlap check via fuzzing with
UndefinedBehaviorSanitizer; the check is added so similar issues can be
caught statically in the future.
Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
|
Why this check is deliberately narrow: This check targets a specific undefined-behavior pattern I hit in real code: a pointer-overlap test of the form I had a broader version of this check but it fired a flood of false positives on completely normal C and since cppcheck prioritizes a low false-positive rate, such a check would just get disabled by users and help no one. |
|
I don't understand why compilers would fold the comparison? even if there is never overflow the comparison is not redundant? |
2 participants
Add a new inconclusive warning that detects the pointer overlap idiom
where p1 and p2 are distinct pointers and n is an unsigned offset, e.g. the memcpy_s overlap check '(dest >= src && dest < src + count) || ...'.
The 'p1 < p2 + n' clause assumes 'p2 + n' does not overflow past the end of the object. If n is large enough that 'p2 + n' wraps (UB), some mainstream compilers assume the wrap cannot happen and fold the comparison, silently dropping the check. The remedy in user code is to cast the pointers to uintptr_t and compare in integer space, with an explicit overflow guard.
To avoid false positives, the warning only fires when the comparison is paired (via &&) with another comparison of the same two pointers, which identifies the overlap idiom; plain bounds checks like 'p < buf + len' and room checks like 'cur + need <= limit' are not flagged. The check is gated behind --enable=inconclusive. The same-pointer case 'p < p + n' is left to the existing invalidTestForOverflow check.
This pattern was found in a real memcpy_s overlap check via fuzzing with UndefinedBehaviorSanitizer; the check is added so similar issues can be caught statically in the future.