From ce6366f023ead4586173dd73918964e65a53ec56 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 7 Mar 2023 14:17:12 +0000 Subject: [PATCH 1/2] C++: Use the parameterized module dataflow API in 'cpp/upcast-array-pointer-arithmetic'. This allows us to swap out the old string state with the Type-based state. --- .../Conversion/CastArrayPointerArithmetic.ql | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql b/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql index 6301b0c8a9a..ddd7d61c360 100644 --- a/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql +++ b/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql @@ -18,31 +18,39 @@ import cpp import semmle.code.cpp.ir.dataflow.DataFlow -import DataFlow::PathGraph +import CastToPointerArithFlow::PathGraph Type getFullyConvertedType(DataFlow::Node node) { result = node.asExpr().getFullyConverted().getUnspecifiedType() } -class CastToPointerArithFlow extends DataFlow::Configuration { - CastToPointerArithFlow() { this = "CastToPointerArithFlow" } +module CastToPointerArithFlowConfig implements DataFlow::StateConfigSig { + class FlowState = Type; - override predicate isSource(DataFlow::Node node, DataFlow::FlowState state) { + predicate isSource(DataFlow::Node node, FlowState state) { not node.asExpr() instanceof Conversion and exists(Type baseType1, Type baseType2 | hasBaseType(node.asExpr(), baseType1) and hasBaseType(node.asExpr().getConversion*(), baseType2) and introducesNewField(baseType1, baseType2) ) and - getFullyConvertedType(node).getName() = state + getFullyConvertedType(node) = state } - override predicate isSink(DataFlow::Node node, DataFlow::FlowState state) { + predicate isSink(DataFlow::Node node, FlowState state) { ( exists(PointerAddExpr pae | pae.getAnOperand() = node.asExpr()) or exists(ArrayExpr ae | ae.getArrayBase() = node.asExpr()) ) and - getFullyConvertedType(node).getName() = state + getFullyConvertedType(node) = state + } + + predicate isBarrier(DataFlow::Node node, FlowState state) { none() } + + predicate isAdditionalFlowStep( + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 + ) { + none() } } @@ -72,14 +80,16 @@ predicate introducesNewField(Class derived, Class base) { ) } +module CastToPointerArithFlow = DataFlow::MakeWithState; + pragma[nomagic] -predicate hasFullyConvertedType(DataFlow::PathNode node, Type t) { +predicate hasFullyConvertedType(CastToPointerArithFlow::PathNode node, Type t) { getFullyConvertedType(node.getNode()) = t } -from DataFlow::PathNode source, DataFlow::PathNode sink, CastToPointerArithFlow cfg, Type t +from CastToPointerArithFlow::PathNode source, CastToPointerArithFlow::PathNode sink, Type t where - cfg.hasFlowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and + CastToPointerArithFlow::hasFlowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and hasFullyConvertedType(source, t) and hasFullyConvertedType(sink, t) select sink, source, sink, "This pointer arithmetic may be done with the wrong type because of $@.", From f2b311a0087a39e4bccfedb254e38f0ed8da32c2 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 7 Mar 2023 14:29:39 +0000 Subject: [PATCH 2/2] C++: We don't need to check type equivalence at the end anymore: the dataflow state now precisely tracks the types. --- .../Conversion/CastArrayPointerArithmetic.ql | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql b/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql index ddd7d61c360..03f622855ab 100644 --- a/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql +++ b/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql @@ -82,15 +82,7 @@ predicate introducesNewField(Class derived, Class base) { module CastToPointerArithFlow = DataFlow::MakeWithState; -pragma[nomagic] -predicate hasFullyConvertedType(CastToPointerArithFlow::PathNode node, Type t) { - getFullyConvertedType(node.getNode()) = t -} - -from CastToPointerArithFlow::PathNode source, CastToPointerArithFlow::PathNode sink, Type t -where - CastToPointerArithFlow::hasFlowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and - hasFullyConvertedType(source, t) and - hasFullyConvertedType(sink, t) +from CastToPointerArithFlow::PathNode source, CastToPointerArithFlow::PathNode sink +where CastToPointerArithFlow::hasFlowPath(source, sink) select sink, source, sink, "This pointer arithmetic may be done with the wrong type because of $@.", source, "this cast"