hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 09:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

C++: Remove the two configurations that depend on flow state to speed up performance on ChakraCore.

Browse Source
This commit is contained in:
Mathias Vorreiter Pedersen
2024-03-11 13:56:22 +00:00
parent 761f6d3a7e
commit 7b0df57d7a
2 changed files with 72 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files

158
cpp/ql/src/Security/CWE/CWE-843/TypeConfusion.ql
View File

@@ -12,7 +12,7 @@
import cpp
import semmle.code.cpp.dataflow.new.DataFlow
import BadFlow::PathGraph
import Flow::PathGraph
/**
* Holds if `f` is a field located at byte offset `offset` in `c`.
@@ -179,23 +179,28 @@ class UnsafeCast extends Cast {
}
/**
* Holds if `source` is an allocation that allocates a value of type `state`.
* Holds if `source` is an allocation that allocates a value of type `type`.
*/
predicate isSourceImpl(DataFlow::Node source, Class state) {
state = source.asExpr().(AllocationExpr).getAllocatedElementType().stripType() and
predicate isSourceImpl(DataFlow::Node source, Class type) {
exists(AllocationExpr alloc |
alloc = source.asExpr() and
type = alloc.getAllocatedElementType().stripType() and
not exists(
alloc
.(NewOrNewArrayExpr)
.getAllocator()
.(OperatorNewAllocationFunction)
.getPlacementArgument()
)
) and
exists(TypeDeclarationEntry tde |
tde = state.getDefinition() and
tde = type.getDefinition() and
not tde.isFromUninstantiatedTemplate(_)
)
}
/**
* The `RelevantStateConfig` configuration is used to find the set of
* states for the `BadConfig` and `GoodConfig`. The flow computed by
* `RelevantStateConfig` is used to implement the `relevantState` predicate
* which is used to avoid a cartesian product in `isSinkImpl`.
*/
module RelevantStateConfig implements DataFlow::ConfigSig {
/** A configuration describing flow from an allocation to a potentially unsafe cast. */
module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { isSourceImpl(source, _) }
predicate isBarrier(DataFlow::Node node) {
@@ -212,122 +217,47 @@ module RelevantStateConfig implements DataFlow::ConfigSig {
)
}
predicate isSink(DataFlow::Node sink) {
exists(UnsafeCast cast | sink.asExpr() = cast.getUnconverted())
}
predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(UnsafeCast cast).getUnconverted() }
int fieldFlowBranchLimit() { result = 0 }
}
module RelevantStateFlow = DataFlow::Global<RelevantStateConfig>;
module Flow = DataFlow::Global<Config>;
predicate relevantState(DataFlow::Node source, DataFlow::Node sink, Class state) {
RelevantStateFlow::flow(source, sink) and
isSourceImpl(source, state)
}
predicate isSinkImpl(DataFlow::Node sink, Class state, Type convertedType, boolean compatible) {
exists(UnsafeCast cast |
relevantState(_, sink, state) and
sink.asExpr() = cast.getUnconverted() and
convertedType = cast.getConvertedType()
|
if cast.compatibleWith(state) then compatible = true else compatible = false
predicate relevantType(DataFlow::Node sink, Class allocatedType) {
exists(DataFlow::Node source |
Flow::flow(source, sink) and
isSourceImpl(source, allocatedType)
)
}
/**
* The `BadConfig` configuration tracks flow from an allocation to an
* incompatible cast.
*
* We use `FlowState` to track the type of the source, and compare the
* flow state to the target of the cast in the `isSink` definition.
*/
module BadConfig implements DataFlow::StateConfigSig {
class FlowState extends Class {
FlowState() { relevantState(_, _, this) }
}
predicate isSource(DataFlow::Node source, FlowState state) { relevantState(source, _, state) }
predicate isBarrier(DataFlow::Node node) { RelevantStateConfig::isBarrier(node) }
predicate isSink(DataFlow::Node sink, FlowState state) { isSinkImpl(sink, state, _, false) }
predicate isBarrierOut(DataFlow::Node sink, FlowState state) { isSink(sink, state) }
int fieldFlowBranchLimit() { result = 0 }
predicate isSinkImpl(
DataFlow::Node sink, Class allocatedType, Type convertedType, boolean compatible
) {
exists(UnsafeCast cast |
relevantType(sink, allocatedType) and
sink.asExpr() = cast.getUnconverted() and
convertedType = cast.getConvertedType()
|
if cast.compatibleWith(allocatedType) then compatible = true else compatible = false
)
}
module BadFlow = DataFlow::GlobalWithState<BadConfig>;
/**
* The `GoodConfig` configuration tracks flow from an allocation to a
* compatible cast.
*
* We use `GoodConfig` to reduce the number of FPs from infeasible paths.
* For example, consider the following example:
* ```cpp
* struct Animal { virtual ~Animal(); };
*
* struct Cat : public Animal {
* Cat();
* ~Cat();
* };
*
* struct Dog : public Animal {
* Dog();
* ~Dog();
* };
*
* void test9(bool b) {
* Animal* a;
* if(b) {
* a = new Cat;
* } else {
* a = new Dog;
* }
* if(b) {
* Cat* d = static_cast<Cat*>(a);
* }
* }
* ```
* Here, `BadConfig` finds a flow from `a = new Dog` to `static_cast<Cat*>(a)`.
* However, that path is never realized in an actual execution path. So in
* order to remove this result we exclude results where there exists an
* allocation of a type that's compatible with `static_cast<Cat*>(a)`.
*
* We use `FlowState` to track the type of the source, and compare the
* flow state to the target of the cast in the `isSink` definition.
*/
module GoodConfig implements DataFlow::StateConfigSig {
class FlowState = BadConfig::FlowState;
predicate isSource(DataFlow::Node source, FlowState state) { BadConfig::isSource(source, state) }
predicate isBarrier(DataFlow::Node node) { BadConfig::isBarrier(node) }
predicate isSink(DataFlow::Node sink, FlowState state) {
isSinkImpl(sink, state, _, true) and
BadFlow::flowTo(sink)
}
int fieldFlowBranchLimit() { result = 0 }
}
module GoodFlow = DataFlow::GlobalWithState<GoodConfig>;
from
BadFlow::PathNode source, BadFlow::PathNode sink, Type sourceType, Type sinkType,
Flow::PathNode source, Flow::PathNode sink, Type badSourceType, Type sinkType,
DataFlow::Node sinkNode
where
BadFlow::flowPath(source, sink) and
Flow::flowPath(source, sink) and
sinkNode = sink.getNode() and
isSourceImpl(source.getNode(), badSourceType) and
isSinkImpl(sinkNode, badSourceType, sinkType, false) and
// If there is any flow that would result in a valid cast then we don't
// report an alert here. This reduces the number of FPs from infeasible paths
// significantly.
not GoodFlow::flowTo(sinkNode) and
isSourceImpl(source.getNode(), sourceType) and
isSinkImpl(sinkNode, _, sinkType, false)
select sinkNode, source, sink, "Conversion from $@ to $@ is invalid.", sourceType,
sourceType.toString(), sinkType, sinkType.toString()
not exists(DataFlow::Node goodSource, Type goodSourceType |
isSourceImpl(goodSource, goodSourceType) and
isSinkImpl(sinkNode, goodSourceType, sinkType, true) and
Flow::flow(goodSource, sinkNode)
)
select sinkNode, source, sink, "Conversion from $@ to $@ is invalid.", badSourceType,
badSourceType.toString(), sinkType, sinkType.toString()

28
cpp/ql/test/query-tests/Security/CWE/CWE-843/TypeConfusion.expected
View File

@@ -1,34 +1,62 @@
edges
| test.cpp:17:13:17:18 | new | test.cpp:18:21:18:47 | p | provenance | |
| test.cpp:22:13:22:26 | new | test.cpp:23:12:23:30 | p | provenance | |
| test.cpp:27:13:27:18 | new | test.cpp:28:25:28:55 | p | provenance | |
| test.cpp:32:13:32:30 | new | test.cpp:33:12:33:30 | p | provenance | |
| test.cpp:47:21:47:36 | new | test.cpp:48:22:48:55 | p | provenance | |
| test.cpp:66:15:66:21 | new | test.cpp:67:12:67:31 | a | provenance | |
| test.cpp:76:15:76:21 | new | test.cpp:77:12:77:31 | a | provenance | |
| test.cpp:83:9:83:15 | new | test.cpp:88:14:88:33 | a | provenance | |
| test.cpp:85:9:85:15 | new | test.cpp:88:14:88:33 | a | provenance | |
| test.cpp:115:12:115:17 | new | test.cpp:116:20:116:51 | s2 | provenance | |
| test.cpp:127:12:127:17 | new | test.cpp:128:24:128:59 | s2 | provenance | |
| test.cpp:140:12:140:17 | new | test.cpp:141:23:141:57 | s1 | provenance | |
| test.cpp:143:14:143:19 | new | test.cpp:145:28:145:68 | s1_2 | provenance | |
| test.cpp:153:9:153:15 | new | test.cpp:159:14:159:33 | a | provenance | |
| test.cpp:166:9:166:15 | new | test.cpp:171:14:171:33 | a | provenance | |
| test.cpp:168:9:168:15 | new | test.cpp:171:14:171:33 | a | provenance | |
| test.cpp:179:15:179:24 | new | test.cpp:181:15:181:25 | u64 | provenance | |
| test.cpp:187:15:187:24 | new | test.cpp:189:25:189:45 | u64 | provenance | |
| test.cpp:207:14:207:26 | new | test.cpp:209:17:209:28 | si | provenance | |
| test.cpp:217:13:217:18 | new | test.cpp:218:30:218:65 | p | provenance | |
| test.cpp:226:13:226:18 | new | test.cpp:227:29:227:63 | p | provenance | |
nodes
| test.cpp:17:13:17:18 | new | semmle.label | new |
| test.cpp:18:21:18:47 | p | semmle.label | p |
| test.cpp:22:13:22:26 | new | semmle.label | new |
| test.cpp:23:12:23:30 | p | semmle.label | p |
| test.cpp:27:13:27:18 | new | semmle.label | new |
| test.cpp:28:25:28:55 | p | semmle.label | p |
| test.cpp:32:13:32:30 | new | semmle.label | new |
| test.cpp:33:12:33:30 | p | semmle.label | p |
| test.cpp:47:21:47:36 | new | semmle.label | new |
| test.cpp:48:22:48:55 | p | semmle.label | p |
| test.cpp:66:15:66:21 | new | semmle.label | new |
| test.cpp:67:12:67:31 | a | semmle.label | a |
| test.cpp:76:15:76:21 | new | semmle.label | new |
| test.cpp:77:12:77:31 | a | semmle.label | a |
| test.cpp:83:9:83:15 | new | semmle.label | new |
| test.cpp:85:9:85:15 | new | semmle.label | new |
| test.cpp:88:14:88:33 | a | semmle.label | a |
| test.cpp:115:12:115:17 | new | semmle.label | new |
| test.cpp:116:20:116:51 | s2 | semmle.label | s2 |
| test.cpp:127:12:127:17 | new | semmle.label | new |
| test.cpp:128:24:128:59 | s2 | semmle.label | s2 |
| test.cpp:140:12:140:17 | new | semmle.label | new |
| test.cpp:141:23:141:57 | s1 | semmle.label | s1 |
| test.cpp:143:14:143:19 | new | semmle.label | new |
| test.cpp:145:28:145:68 | s1_2 | semmle.label | s1_2 |
| test.cpp:153:9:153:15 | new | semmle.label | new |
| test.cpp:159:14:159:33 | a | semmle.label | a |
| test.cpp:166:9:166:15 | new | semmle.label | new |
| test.cpp:168:9:168:15 | new | semmle.label | new |
| test.cpp:171:14:171:33 | a | semmle.label | a |
| test.cpp:179:15:179:24 | new | semmle.label | new |
| test.cpp:181:15:181:25 | u64 | semmle.label | u64 |
| test.cpp:187:15:187:24 | new | semmle.label | new |
| test.cpp:189:25:189:45 | u64 | semmle.label | u64 |
| test.cpp:207:14:207:26 | new | semmle.label | new |
| test.cpp:209:17:209:28 | si | semmle.label | si |
| test.cpp:217:13:217:18 | new | semmle.label | new |
| test.cpp:218:30:218:65 | p | semmle.label | p |
| test.cpp:226:13:226:18 | new | semmle.label | new |