Skip to content

Commit f5062c7

Browse files
committed
C++: Remove a bunch of bad self joins from 'cpp/toctou-race-condition'.
1 parent e96fcf8 commit f5062c7

File tree

1 file changed

+52
-30
lines changed

1 file changed

+52
-30
lines changed

cpp/ql/src/Security/CWE/CWE-367/TOCTOUFilesystemRace.ql

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -89,44 +89,66 @@ predicate referenceTo(Expr source, Expr use) {
8989
)
9090
}
9191

92+
pragma[noinline]
93+
predicate statCallWithPointer(Expr checkPath, Expr call, Expr e, Variable v) {
94+
call = stat(checkPath, e) and
95+
e.getAChild*().(VariableAccess).getTarget() = v
96+
}
97+
98+
predicate checksPath(Expr check, Expr checkPath) {
99+
// either:
100+
// an access check
101+
check = accessCheck(checkPath)
102+
or
103+
// a stat
104+
check = stat(checkPath, _)
105+
or
106+
// access to a member variable on the stat buf
107+
// (morally, this should be a use-use pair, but it seems unlikely
108+
// that this variable will get reused in practice)
109+
exists(Expr call, Expr e, Variable v |
110+
statCallWithPointer(checkPath, call, e, v) and
111+
check.(VariableAccess).getTarget() = v and
112+
not e.getAChild*() = check // the call that writes to the pointer is not where the pointer is checked.
113+
)
114+
}
115+
116+
predicate checkUse(Expr check, Expr checkPath, FunctionCall use, Expr usePath) {
117+
// `check` looks like a check on a filename
118+
checksPath(check, checkPath) and
119+
// `op` looks like an operation on a filename
120+
use = filenameOperation(usePath)
121+
or
122+
// another filename operation (null pointers can indicate errors)
123+
check = filenameOperation(checkPath) and
124+
// `op` looks like a sensitive operation on a filename
125+
use = sensitiveFilenameOperation(usePath)
126+
}
127+
128+
pragma[noinline]
129+
Expr getACheckedPath(Expr check, SsaDefinition def, StackVariable v) {
130+
checkUse(check, result, _, _) and
131+
def.getAUse(v) = result
132+
}
133+
134+
pragma[noinline]
135+
Expr getAUsedPath(FunctionCall use, SsaDefinition def, StackVariable v) {
136+
checkUse(_, _, use, result) and
137+
def.getAUse(v) = result
138+
}
139+
92140
from Expr check, Expr checkPath, FunctionCall use, Expr usePath
93141
where
94-
// `check` looks like a check on a filename
95-
(
96-
(
97-
// either:
98-
// an access check
99-
check = accessCheck(checkPath)
100-
or
101-
// a stat
102-
check = stat(checkPath, _)
103-
or
104-
// access to a member variable on the stat buf
105-
// (morally, this should be a use-use pair, but it seems unlikely
106-
// that this variable will get reused in practice)
107-
exists(Expr call, Expr e, Variable v |
108-
call = stat(checkPath, e) and
109-
e.getAChild*().(VariableAccess).getTarget() = v and
110-
check.(VariableAccess).getTarget() = v and
111-
not e.getAChild*() = check // the call that writes to the pointer is not where the pointer is checked.
112-
)
113-
) and
114-
// `op` looks like an operation on a filename
115-
use = filenameOperation(usePath)
116-
or
117-
// another filename operation (null pointers can indicate errors)
118-
check = filenameOperation(checkPath) and
119-
// `op` looks like a sensitive operation on a filename
120-
use = sensitiveFilenameOperation(usePath)
121-
) and
142+
checkUse(check, checkPath, use, usePath) and
122143
// `checkPath` and `usePath` refer to the same SSA variable
123144
exists(SsaDefinition def, StackVariable v |
124-
def.getAUse(v) = checkPath and def.getAUse(v) = usePath
145+
getACheckedPath(check, def, v) = checkPath and
146+
getAUsedPath(use, def, v) = usePath
125147
) and
126148
// the return value of `check` is used (possibly with one step of
127149
// variable indirection) in a guard which controls `use`
128150
exists(GuardCondition guard | referenceTo(check, guard.getAChild*()) |
129-
guard.controls(use.(ControlFlowNode).getBasicBlock(), _)
151+
guard.controls(use.getBasicBlock(), _)
130152
)
131153
select use,
132154
"The $@ being operated upon was previously $@, but the underlying file may have been changed since then.",

0 commit comments

Comments
 (0)