Skip to content

Commit 2af48e2

Browse files
authored
Merge pull request github#12970 from MathiasVP/barrier-guards-for-indirect-expressions
C++: Barrier guards API for indirect expressions
2 parents f8ef697 + 2d98fb7 commit 2af48e2

File tree

4 files changed

+136
-7
lines changed

4 files changed

+136
-7
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* 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.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1903,7 +1903,38 @@ signature predicate guardChecksSig(IRGuardCondition g, Expr e, boolean branch);
19031903
* in data flow and taint tracking.
19041904
*/
19051905
module BarrierGuard<guardChecksSig/3 guardChecks> {
1906-
/** Gets a node that is safely guarded by the given guard check. */
1906+
/**
1907+
* Gets an expression node that is safely guarded by the given guard check.
1908+
*
1909+
* For example, given the following code:
1910+
* ```cpp
1911+
* int x = source();
1912+
* // ...
1913+
* if(is_safe_int(x)) {
1914+
* sink(x);
1915+
* }
1916+
* ```
1917+
* and the following barrier guard predicate:
1918+
* ```ql
1919+
* predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
1920+
* exists(Call call |
1921+
* g.getUnconvertedResultExpression() = call and
1922+
* call.getTarget().hasName("is_safe_int") and
1923+
* e = call.getAnArgument() and
1924+
* branch = true
1925+
* )
1926+
* }
1927+
* ```
1928+
* implementing `isBarrier` as:
1929+
* ```ql
1930+
* predicate isBarrier(DataFlow::Node barrier) {
1931+
* barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getABarrierNode()
1932+
* }
1933+
* ```
1934+
* will block flow from `x = source()` to `sink(x)`.
1935+
*
1936+
* NOTE: If an indirect expression is tracked, use `getAnIndirectBarrierNode` instead.
1937+
*/
19071938
ExprNode getABarrierNode() {
19081939
exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
19091940
e = value.getAnInstruction().getConvertedResultExpression() and
@@ -1912,6 +1943,84 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
19121943
g.controls(result.getBasicBlock(), edge)
19131944
)
19141945
}
1946+
1947+
/**
1948+
* Gets an indirect expression node that is safely guarded by the given guard check.
1949+
*
1950+
* For example, given the following code:
1951+
* ```cpp
1952+
* int* p;
1953+
* // ...
1954+
* *p = source();
1955+
* if(is_safe_pointer(p)) {
1956+
* sink(*p);
1957+
* }
1958+
* ```
1959+
* and the following barrier guard check:
1960+
* ```ql
1961+
* predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
1962+
* exists(Call call |
1963+
* g.getUnconvertedResultExpression() = call and
1964+
* call.getTarget().hasName("is_safe_pointer") and
1965+
* e = call.getAnArgument() and
1966+
* branch = true
1967+
* )
1968+
* }
1969+
* ```
1970+
* implementing `isBarrier` as:
1971+
* ```ql
1972+
* predicate isBarrier(DataFlow::Node barrier) {
1973+
* barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getAnIndirectBarrierNode()
1974+
* }
1975+
* ```
1976+
* will block flow from `x = source()` to `sink(x)`.
1977+
*
1978+
* NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
1979+
*/
1980+
IndirectExprNode getAnIndirectBarrierNode() { result = getAnIndirectBarrierNode(_) }
1981+
1982+
/**
1983+
* Gets an indirect expression node with indirection index `indirectionIndex` that is
1984+
* safely guarded by the given guard check.
1985+
*
1986+
* For example, given the following code:
1987+
* ```cpp
1988+
* int* p;
1989+
* // ...
1990+
* *p = source();
1991+
* if(is_safe_pointer(p)) {
1992+
* sink(*p);
1993+
* }
1994+
* ```
1995+
* and the following barrier guard check:
1996+
* ```ql
1997+
* predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
1998+
* exists(Call call |
1999+
* g.getUnconvertedResultExpression() = call and
2000+
* call.getTarget().hasName("is_safe_pointer") and
2001+
* e = call.getAnArgument() and
2002+
* branch = true
2003+
* )
2004+
* }
2005+
* ```
2006+
* implementing `isBarrier` as:
2007+
* ```ql
2008+
* predicate isBarrier(DataFlow::Node barrier) {
2009+
* barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getAnIndirectBarrierNode(1)
2010+
* }
2011+
* ```
2012+
* will block flow from `x = source()` to `sink(x)`.
2013+
*
2014+
* NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
2015+
*/
2016+
IndirectExprNode getAnIndirectBarrierNode(int indirectionIndex) {
2017+
exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
2018+
e = value.getAnInstruction().getConvertedResultExpression() and
2019+
result.getConvertedExpr(indirectionIndex) = e and
2020+
guardChecks(g, value.getAnInstruction().getConvertedResultExpression(), edge) and
2021+
g.controls(result.getBasicBlock(), edge)
2022+
)
2023+
}
19152024
}
19162025

19172026
/**

cpp/ql/test/library-tests/dataflow/dataflow-tests/BarrierGuard.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
int source();
2-
void sink(int);
2+
void sink(...);
33
bool guarded(int);
44

55
void bg_basic(int source) {
@@ -66,3 +66,13 @@ void bg_structptr(XY *p1, XY *p2) { // $ ast-def=p1 ast-def=p2
6666
sink(p1->x); // $ ast,ir
6767
}
6868
}
69+
70+
int* indirect_source();
71+
bool guarded(const int*);
72+
73+
void bg_indirect_expr() {
74+
int *buf = indirect_source();
75+
if (guarded(buf)) {
76+
sink(buf);
77+
}
78+
}

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.ql

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ module AstTest {
4747
}
4848

4949
module IRTest {
50+
private import cpp
5051
private import semmle.code.cpp.ir.dataflow.DataFlow
5152
private import semmle.code.cpp.ir.IR
5253
private import semmle.code.cpp.controlflow.IRGuards
@@ -56,10 +57,13 @@ module IRTest {
5657
* S in `if (guarded(x)) S`.
5758
*/
5859
// This is tested in `BarrierGuard.cpp`.
59-
predicate testBarrierGuard(IRGuardCondition g, Instruction checked, boolean isTrue) {
60-
g.(CallInstruction).getStaticCallTarget().getName() = "guarded" and
61-
checked = g.(CallInstruction).getPositionalArgument(0) and
62-
isTrue = true
60+
predicate testBarrierGuard(IRGuardCondition g, Expr checked, boolean isTrue) {
61+
exists(Call call |
62+
call = g.getUnconvertedResultExpression() and
63+
call.getTarget().hasName("guarded") and
64+
checked = call.getArgument(0) and
65+
isTrue = true
66+
)
6367
}
6468

6569
/** Common data flow configuration to be used by tests. */
@@ -90,7 +94,9 @@ module IRTest {
9094
barrierExpr.(VariableAccess).getTarget().hasName("barrier")
9195
)
9296
or
93-
barrier = DataFlow::InstructionBarrierGuard<testBarrierGuard/3>::getABarrierNode()
97+
barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getABarrierNode()
98+
or
99+
barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getAnIndirectBarrierNode()
94100
}
95101
}
96102
}

0 commit comments

Comments
 (0)