Skip to content

Commit fd4967e

Browse files
committed
C++: Fix SnprintfOverflow issues
Requiring strict inclusion between types turned out to cause false positives in `SnprintfOverflow`, which relied indirectly on `RangeAnalysisUtils::linearAccessImpl` to identify acceptable bounds checks. This query was particularly affected because `snprintf` returns `int` (signed) but takes `size_t` (unsigned), so conversions are bound to happen.
1 parent 93286aa commit fd4967e

File tree

2 files changed

+4
-3
lines changed

2 files changed

+4
-3
lines changed

cpp/ql/src/semmle/code/cpp/rangeanalysis/RangeAnalysisUtils.qll

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,10 @@ predicate linearAccessImpl(Expr expr, VariableAccess v, float p, float q) {
209209
| linearAccess(cast.getExpr(), v, p, q) and
210210
sourceType = cast.getExpr().getType().getUnspecifiedType() and
211211
targetType = cast.getType().getUnspecifiedType() and
212-
typeLowerBound(targetType) <= typeLowerBound(sourceType) and
213-
typeUpperBound(targetType) >= typeUpperBound(sourceType) and
212+
// This allows conversion between signed and unsigned, which is technically
213+
// lossy but common enough that we'll just have to assume the user knows
214+
// what they're doing.
215+
targetType.getSize() >= sourceType.getSize() and
214216
expr = cast)
215217
or
216218
// (p*v+q) == p*v + q

cpp/ql/test/query-tests/Likely Bugs/Format/SnprintfOverflow/SnprintfOverflow.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,3 @@
22
| test.cpp:62:12:62:19 | call to snprintf | The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow. | test.cpp:62:26:62:34 | remaining | size argument |
33
| test.cpp:76:10:76:17 | call to snprintf | The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow. | test.cpp:76:24:76:32 | ... - ... | size argument |
44
| test.cpp:100:10:100:19 | call to snprintf_s | The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow. | test.cpp:100:35:100:54 | ... - ... | size argument |
5-
| test.cpp:109:15:109:22 | call to snprintf | The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow. | test.cpp:109:29:109:35 | buf_len | size argument |

0 commit comments

Comments
 (0)