mirror of
https://github.com/github/codeql.git
synced 2026-04-28 02:05:14 +02:00
C++: Taint through most partial chi operands
This changes the flow to be taint rather than data flow, and it extends it to include chi instructions with unknown type as long as they're not for the `AliasedVirtualVariable`. We're losing three good test results because these tests are not affected by `DefaultTaintTracking.qll`. The taint step added here can later be ported to `TaintTrackingUtil.qll` to recover these results, but we probably want a better API than transitive-closure search through instructions before doing that.
This commit is contained in:
@@ -149,6 +149,9 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
|
||||
or
|
||||
i2.(UnaryInstruction).getUnary() = i1
|
||||
or
|
||||
i2.(ChiInstruction).getPartial() = i1 and
|
||||
not isChiForAllAliasedMemory(i2)
|
||||
or
|
||||
exists(BinaryInstruction bin |
|
||||
bin = i2 and
|
||||
predictableInstruction(i2.getAnOperand().getDef()) and
|
||||
@@ -205,6 +208,19 @@ private predicate modelTaintToParameter(Function f, int parameterIn, int paramet
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `chi` is on the chain of chi-instructions for all aliased memory.
|
||||
* Taint shoud not pass through these instructions since they tend to mix up
|
||||
* unrelated objects.
|
||||
*/
|
||||
private predicate isChiForAllAliasedMemory(Instruction instr) {
|
||||
instr.(ChiInstruction).getTotal() instanceof AliasedDefinitionInstruction
|
||||
or
|
||||
isChiForAllAliasedMemory(instr.(ChiInstruction).getTotal())
|
||||
or
|
||||
isChiForAllAliasedMemory(instr.(PhiInstruction).getAnInput())
|
||||
}
|
||||
|
||||
private predicate modelTaintToReturnValue(Function f, int parameterIn) {
|
||||
// Taint flow from parameter to return value
|
||||
exists(FunctionInput modelIn, FunctionOutput modelOut |
|
||||
|
||||
@@ -283,18 +283,10 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
|
||||
// By allowing flow through the total operand, we ensure that flow is not lost
|
||||
// due to shortcomings of the alias analysis. We may get false flow in cases
|
||||
// where the data is indeed overwritten.
|
||||
//
|
||||
// Flow through the partial operand belongs in the taint-tracking libraries
|
||||
// for now.
|
||||
iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom
|
||||
or
|
||||
// Flow through the partial operand must be restricted a bit more. For
|
||||
// soundness, the IR has to assume that every write to an unknown address can
|
||||
// affect every escaped variable, and this assumption shows up as data flowing
|
||||
// through partial chi operands. The chi instructions for all escaped data can
|
||||
// be recognized by having unknown types. For all other chi instructions, flow
|
||||
// through partial operands is more likely to be real.
|
||||
exists(ChiInstruction chi | iTo = chi |
|
||||
iFrom = chi.getPartial() and
|
||||
not chi.getResultIRType() instanceof IRUnknownType
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -107,9 +107,9 @@ void array_test(int i) {
|
||||
arr3[5] = 0;
|
||||
|
||||
sink(arr1[5]); // tainted
|
||||
sink(arr1[i]); // tainted [NOT DETECTED with AST]
|
||||
sink(arr2[5]); // tainted [NOT DETECTED with AST]
|
||||
sink(arr2[i]); // tainted [NOT DETECTED with AST]
|
||||
sink(arr1[i]); // tainted [NOT DETECTED]
|
||||
sink(arr2[5]); // tainted [NOT DETECTED]
|
||||
sink(arr2[i]); // tainted [NOT DETECTED]
|
||||
sink(arr3[5]);
|
||||
sink(arr3[i]);
|
||||
}
|
||||
|
||||
@@ -17,9 +17,6 @@
|
||||
| taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
|
||||
| taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
|
||||
| taint.cpp:109:7:109:13 | taint.cpp:105:12:105:17 | IR only |
|
||||
| taint.cpp:110:7:110:13 | taint.cpp:105:12:105:17 | IR only |
|
||||
| taint.cpp:111:7:111:13 | taint.cpp:106:12:106:17 | IR only |
|
||||
| taint.cpp:112:7:112:13 | taint.cpp:106:12:106:17 | IR only |
|
||||
| taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
|
||||
| taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
|
||||
| taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |
|
||||
|
||||
@@ -2,9 +2,6 @@
|
||||
| taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
|
||||
| taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
|
||||
| taint.cpp:109:7:109:13 | access to array | taint.cpp:105:12:105:17 | call to source |
|
||||
| taint.cpp:110:7:110:13 | access to array | taint.cpp:105:12:105:17 | call to source |
|
||||
| taint.cpp:111:7:111:13 | access to array | taint.cpp:106:12:106:17 | call to source |
|
||||
| taint.cpp:112:7:112:13 | access to array | taint.cpp:106:12:106:17 | call to source |
|
||||
| taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
|
||||
| taint.cpp:130:7:130:9 | * ... | taint.cpp:127:8:127:13 | call to source |
|
||||
| taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |
|
||||
|
||||
Reference in New Issue
Block a user