Skip to content

Commit

Permalink
C++: Reuse bounded predicate in TaintedAllocationSize query
Browse files Browse the repository at this point in the history
  • Loading branch information
paldepind committed Aug 14, 2024
1 parent 4e02e34 commit 668a02e
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 10 deletions.
4 changes: 4 additions & 0 deletions cpp/ql/src/Security/CWE/CWE-190/Bounded.qll
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ private predicate boundedBitwiseAnd(Expr e, Expr andExpr, Expr operand1, Expr op
* operation that may greatly reduce the range of possible values.
*/
predicate bounded(Expr e) {
// There can be two separate reasons for `convertedExprMightOverflow` not holding:
// 1. `e` really cannot overflow.
// 2. `e` isn't analyzable.
// If we didn't rule out case 2 we would declare anything that isn't analyzable as bounded.
(
e instanceof UnaryArithmeticOperation or
e instanceof BinaryArithmeticOperation or
Expand Down
12 changes: 2 additions & 10 deletions cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import semmle.code.cpp.ir.IR
import semmle.code.cpp.controlflow.IRGuards
import semmle.code.cpp.security.FlowSources
import TaintedAllocationSize::PathGraph
import Bounded

/**
* Holds if `alloc` is an allocation, and `tainted` is a child of it that is a
Expand Down Expand Up @@ -61,16 +62,7 @@ module TaintedAllocationSizeConfig implements DataFlow::ConfigSig {

predicate isBarrier(DataFlow::Node node) {
exists(Expr e | e = node.asExpr() |
// There can be two separate reasons for `convertedExprMightOverflow` not holding:
// 1. `e` really cannot overflow.
// 2. `e` isn't analyzable.
// If we didn't rule out case 2 we would place barriers on anything that isn't analyzable.
(
e instanceof UnaryArithmeticOperation or
e instanceof BinaryArithmeticOperation or
e instanceof AssignArithmeticOperation
) and
not convertedExprMightOverflow(e)
bounded(e)
or
// Subtracting two pointers is either well-defined (and the result will likely be small), or
// terribly undefined and dangerous. Here, we assume that the programmer has ensured that the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,15 @@ void more_bounded_tests() {
}
}

{
int size = atoi(getenv("USER"));

if (size % 100)
{
malloc(size * sizeof(int)); // GOOD
}
}

{
int size = atoi(getenv("USER"));

Expand Down

0 comments on commit 668a02e

Please sign in to comment.