[zlaski/pointer-overflow-check] Refine query to exclude macros (other than 'assert').

This commit is contained in:
Ziemowit Laski
2019-11-05 18:25:41 -08:00
parent 0df3d2ce60
commit 1f82ea7750
3 changed files with 22 additions and 12 deletions

View File

@@ -1,3 +1,3 @@
bool check_pointer_overflow(P *ptr, P *ptr_end) {
return ptr + 4 >= ptr_end; // GOOD
return ptr_end - ptr >= 4; // GOOD
}

View File

@@ -4,7 +4,7 @@
<qhelp>
<overview>
<p>
When checking for out-of-range pointer values, one might write tests like
When checking for out-of-range pointer values, we might write tests like
<code>p + a &lt; p</code> and check if the value "wraps around".
Such a test is wrong in that it relies on the overflow of <code>p + a</code>,
which has undefined behavior. In fact, many optimizing compilers will remove
@@ -18,13 +18,12 @@ the operating system) or nonexistent.
<recommendation>
<p>
When checking for an out-of-range pointer, compare the pointer
value <code>p</code> against a known value <code>p_max</code> representing
the highest allowable memory address. Ideally, <code>p_max</code> should
point just past the end of a data structure, such as an array or a vector.
For lower-level work, it should point at the highest address in the program
heap. It is also important that <code>p + a</code> points at a valid object
and that <code>a</code> is small enough so that the expression <code>p + a</code>
does not itself overflow.
value <code>p</code> against a known pointer value <code>p_max</code> representing
the highest allowable memory address. The <code>p_max</code> pointer should
point inside or just past the end of a defined memory region, such that
<code>p + a &lt;= p_max</code>. Since we do not wish to evaluate <code>p + a</code>
(which may result in undefined behavior), we rewrite the inequality as
<code>p_max - p &gt; a</code>.
</p>
</recommendation>
<example>

View File

@@ -3,7 +3,7 @@
* @description Testing for out-of-range pointers by adding a value to a pointer
* to see if it "wraps around" is dangerous because it relies
* on undefined behavior and may lead to attempted use of
* nonexistent or inaccessible memory locations
* nonexistent or inaccessible memory locations.
* @kind problem
* @problem.severity warning
* @precision high
@@ -14,12 +14,23 @@
import cpp
private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
class AssertMacro extends Macro {
AssertMacro() {
getName() = "assert" or
getName() = "_ASSERT" or
getName() = "_ASSERTE" or
getName() = "_ASSERT_EXPR"
}
}
from RelationalOperation ro, PointerAddExpr add, Expr expr1, Expr expr2
where
ro.getAnOperand() = add and
add.getAnOperand() = expr1 and
ro.getAnOperand() = expr2 and
globalValueNumber(expr1) = globalValueNumber(expr2)
globalValueNumber(expr1) = globalValueNumber(expr2) and
not exists(MacroInvocation mi |
mi.getAnAffectedElement() = add and not mi.getMacro() instanceof AssertMacro
)
select ro, "Pointer out-of-range check relying on pointer overflow is undefined."