Merge pull request #12970 from MathiasVP/barrier-guards-for-indirect-expressions

C++: Barrier guards API for indirect expressions
This commit is contained in:
Mathias Vorreiter Pedersen
2023-05-03 18:14:01 +01:00
committed by GitHub
4 changed files with 136 additions and 7 deletions

View File

@@ -0,0 +1,4 @@
---
category: feature
---
* A new predicate `BarrierGuard::getAnIndirectBarrierNode` has been added to the new dataflow library (`semmle.code.cpp.dataflow.new.DataFlow`) to mark indirect expressions as barrier nodes using the `BarrierGuard` API.

View File

@@ -1903,7 +1903,38 @@ signature predicate guardChecksSig(IRGuardCondition g, Expr e, boolean branch);
* in data flow and taint tracking.
*/
module BarrierGuard<guardChecksSig/3 guardChecks> {
/** Gets a node that is safely guarded by the given guard check. */
/**
* Gets an expression node that is safely guarded by the given guard check.
*
* For example, given the following code:
* ```cpp
* int x = source();
* // ...
* if(is_safe_int(x)) {
* sink(x);
* }
* ```
* and the following barrier guard predicate:
* ```ql
* predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
* exists(Call call |
* g.getUnconvertedResultExpression() = call and
* call.getTarget().hasName("is_safe_int") and
* e = call.getAnArgument() and
* branch = true
* )
* }
* ```
* implementing `isBarrier` as:
* ```ql
* predicate isBarrier(DataFlow::Node barrier) {
* barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getABarrierNode()
* }
* ```
* will block flow from `x = source()` to `sink(x)`.
*
* NOTE: If an indirect expression is tracked, use `getAnIndirectBarrierNode` instead.
*/
ExprNode getABarrierNode() {
exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
e = value.getAnInstruction().getConvertedResultExpression() and
@@ -1912,6 +1943,84 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
g.controls(result.getBasicBlock(), edge)
)
}
/**
* Gets an indirect expression node that is safely guarded by the given guard check.
*
* For example, given the following code:
* ```cpp
* int* p;
* // ...
* *p = source();
* if(is_safe_pointer(p)) {
* sink(*p);
* }
* ```
* and the following barrier guard check:
* ```ql
* predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
* exists(Call call |
* g.getUnconvertedResultExpression() = call and
* call.getTarget().hasName("is_safe_pointer") and
* e = call.getAnArgument() and
* branch = true
* )
* }
* ```
* implementing `isBarrier` as:
* ```ql
* predicate isBarrier(DataFlow::Node barrier) {
* barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getAnIndirectBarrierNode()
* }
* ```
* will block flow from `x = source()` to `sink(x)`.
*
* NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
*/
IndirectExprNode getAnIndirectBarrierNode() { result = getAnIndirectBarrierNode(_) }
/**
* Gets an indirect expression node with indirection index `indirectionIndex` that is
* safely guarded by the given guard check.
*
* For example, given the following code:
* ```cpp
* int* p;
* // ...
* *p = source();
* if(is_safe_pointer(p)) {
* sink(*p);
* }
* ```
* and the following barrier guard check:
* ```ql
* predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
* exists(Call call |
* g.getUnconvertedResultExpression() = call and
* call.getTarget().hasName("is_safe_pointer") and
* e = call.getAnArgument() and
* branch = true
* )
* }
* ```
* implementing `isBarrier` as:
* ```ql
* predicate isBarrier(DataFlow::Node barrier) {
* barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getAnIndirectBarrierNode(1)
* }
* ```
* will block flow from `x = source()` to `sink(x)`.
*
* NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
*/
IndirectExprNode getAnIndirectBarrierNode(int indirectionIndex) {
exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
e = value.getAnInstruction().getConvertedResultExpression() and
result.getConvertedExpr(indirectionIndex) = e and
guardChecks(g, value.getAnInstruction().getConvertedResultExpression(), edge) and
g.controls(result.getBasicBlock(), edge)
)
}
}
/**

View File

@@ -1,5 +1,5 @@
int source();
void sink(int);
void sink(...);
bool guarded(int);
void bg_basic(int source) {
@@ -66,3 +66,13 @@ void bg_structptr(XY *p1, XY *p2) { // $ ast-def=p1 ast-def=p2
sink(p1->x); // $ ast,ir
}
}
int* indirect_source();
bool guarded(const int*);
void bg_indirect_expr() {
int *buf = indirect_source();
if (guarded(buf)) {
sink(buf);
}
}

View File

@@ -47,6 +47,7 @@ module AstTest {
}
module IRTest {
private import cpp
private import semmle.code.cpp.ir.dataflow.DataFlow
private import semmle.code.cpp.ir.IR
private import semmle.code.cpp.controlflow.IRGuards
@@ -56,10 +57,13 @@ module IRTest {
* S in `if (guarded(x)) S`.
*/
// This is tested in `BarrierGuard.cpp`.
predicate testBarrierGuard(IRGuardCondition g, Instruction checked, boolean isTrue) {
g.(CallInstruction).getStaticCallTarget().getName() = "guarded" and
checked = g.(CallInstruction).getPositionalArgument(0) and
isTrue = true
predicate testBarrierGuard(IRGuardCondition g, Expr checked, boolean isTrue) {
exists(Call call |
call = g.getUnconvertedResultExpression() and
call.getTarget().hasName("guarded") and
checked = call.getArgument(0) and
isTrue = true
)
}
/** Common data flow configuration to be used by tests. */
@@ -90,7 +94,9 @@ module IRTest {
barrierExpr.(VariableAccess).getTarget().hasName("barrier")
)
or
barrier = DataFlow::InstructionBarrierGuard<testBarrierGuard/3>::getABarrierNode()
barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getABarrierNode()
or
barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getAnIndirectBarrierNode()
}
}
}