diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 1185b6a0c9c3..55325f055ce6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -156,7 +156,7 @@ class Node extends TIRDataFlowNode { * If `isGLValue()` holds, then the type of this node * should be thought of as "pointer to `getType()`". */ - DataFlowType getType() { none() } // overridden in subclasses + Type getType() { none() } // overridden in subclasses /** Gets the instruction corresponding to this node, if any. */ Instruction asInstruction() { result = this.(InstructionNode).getInstruction() } @@ -541,7 +541,7 @@ class Node extends TIRDataFlowNode { /** * Gets an upper bound on the type of this node. */ - DataFlowType getTypeBound() { result = this.getType() } + Type getTypeBound() { result = this.getType() } /** Gets the location of this element. */ cached @@ -585,7 +585,7 @@ private class Node0 extends Node, TNode0 { override string toStringImpl() { result = node.toString() } - override DataFlowType getType() { result = node.getType() } + override Type getType() { result = node.getType() } override predicate isGLValue() { node.isGLValue() } } @@ -704,7 +704,7 @@ class SsaSynthNode extends Node, TSsaSynthNode { override Declaration getFunction() { result = node.getBasicBlock().getEnclosingFunction() } - override DataFlowType getType() { result = node.getSourceVariable().getType() } + override Type getType() { result = node.getSourceVariable().getType() } override predicate isGLValue() { node.getSourceVariable().isGLValue() } @@ -732,7 +732,7 @@ class SsaIteratorNode extends Node, TSsaIteratorNode { override Declaration getFunction() { result = node.getFunction() } - override DataFlowType getType() { result = node.getType() } + override Type getType() { result = node.getType() } final override Location getLocationImpl() { result = node.getLocation() } @@ -792,7 +792,7 @@ class FinalGlobalValue extends Node, TFinalGlobalValue { override Declaration getFunction() { result = globalUse.getIRFunction().getFunction() } - override DataFlowType getType() { + override Type getType() { exists(int indirectionIndex | indirectionIndex = globalUse.getIndirectionIndex() and result = getTypeImpl(globalUse.getUnderlyingType(), indirectionIndex) @@ -826,7 +826,7 @@ class InitialGlobalValue extends Node, TInitialGlobalValue { final override predicate isGLValue() { globalDef.getIndirectionIndex() = 0 } - override DataFlowType getType() { result = globalDef.getUnderlyingType() } + override Type getType() { result = globalDef.getUnderlyingType() } final override Location getLocationImpl() { result = globalDef.getLocation() } @@ -853,7 +853,7 @@ class BodyLessParameterNodeImpl extends Node, TBodyLessParameterNodeImpl { /** Gets the indirection index of this node. */ int getIndirectionIndex() { result = indirectionIndex } - override DataFlowType getType() { + override Type getType() { result = getTypeImpl(p.getUnderlyingType(), this.getIndirectionIndex()) } @@ -1117,8 +1117,8 @@ private module RawIndirectNodes { override predicate isGLValue() { this.getOperand().isGLValue() } - override DataFlowType getType() { - exists(int sub, DataFlowType type, boolean isGLValue | + override Type getType() { + exists(int sub, Type type, boolean isGLValue | type = getOperandType(this.getOperand(), isGLValue) and if isGLValue = true then sub = 1 else sub = 0 | @@ -1163,7 +1163,7 @@ private module RawIndirectNodes { override predicate isGLValue() { this.getInstruction().isGLValue() } - override DataFlowType getType() { + override Type getType() { exists(int sub, DataFlowType type, boolean isGLValue | type = getInstructionType(this.getInstruction(), isGLValue) and if isGLValue = true then sub = 1 else sub = 0 @@ -1263,7 +1263,7 @@ class FinalParameterNode extends Node, TFinalParameterNode { result.asSourceCallable() = this.getFunction() } - override DataFlowType getType() { result = getTypeImpl(p.getUnderlyingType(), indirectionIndex) } + override Type getType() { result = getTypeImpl(p.getUnderlyingType(), indirectionIndex) } final override Location getLocationImpl() { // Parameters can have multiple locations. When there's a unique location we use @@ -1539,7 +1539,7 @@ abstract class PostUpdateNode extends Node { */ abstract Node getPreUpdateNode(); - final override DataFlowType getType() { result = this.getPreUpdateNode().getType() } + final override Type getType() { result = this.getPreUpdateNode().getType() } } /** @@ -1632,9 +1632,7 @@ class VariableNode extends Node, TGlobalLikeVariableNode { result.asSourceCallable() = v } - override DataFlowType getType() { - result = getTypeImpl(v.getUnderlyingType(), indirectionIndex - 1) - } + override Type getType() { result = getTypeImpl(v.getUnderlyingType(), indirectionIndex - 1) } final override Location getLocationImpl() { // Certain variables (such as parameters) can have multiple locations. diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll index 8dc3513b444b..c4f5885cf6d4 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll @@ -1064,8 +1064,15 @@ module BarrierGuardWithIntParam { DataFlowIntegrationInput::Guard g, SsaImpl::Definition def, IRGuards::GuardValue val, int indirectionIndex ) { - IRGuards::Guards_v1::ParameterizedValidationWrapper::guardChecksDef(g, - def, val, indirectionIndex) + exists(Instruction e | + IRGuards::Guards_v1::ParameterizedValidationWrapper::guardChecks(g, + e, val, indirectionIndex) + | + indirectionIndex = 0 and + def.(Definition).getAUse().getDef() = e + or + def.(Definition).getAnIndirectUse(indirectionIndex).getDef() = e + ) } Node getABarrierNode(int indirectionIndex) { diff --git a/cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql b/cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql index 4d33ede93150..8cb0122b668e 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql +++ b/cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql @@ -122,7 +122,8 @@ module Config implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { // Block flow if the node is guarded by any <, <= or = operations. - node = DataFlow::BarrierGuard::getABarrierNode() + node = DataFlow::BarrierGuard::getABarrierNode() or + node = DataFlow::BarrierGuard::getAnIndirectBarrierNode() } predicate observeDiffInformedIncrementalMode() { any() } diff --git a/cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll index 28892c5b8207..ba1221d112b7 100644 --- a/cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll +++ b/cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll @@ -78,7 +78,7 @@ module ModelGeneratorCommonInput implements ModelGeneratorCommonInputSig indirect_barrier=int } } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/ir-barrier-guards/test.ql b/cpp/ql/test/library-tests/dataflow/ir-barrier-guards/test.ql index 20610c55385c..5961da371228 100644 --- a/cpp/ql/test/library-tests/dataflow/ir-barrier-guards/test.ql +++ b/cpp/ql/test/library-tests/dataflow/ir-barrier-guards/test.ql @@ -13,26 +13,33 @@ predicate instructionGuardChecks(IRGuardCondition gc, Instruction checked, boole module BarrierGuard = DataFlow::InstructionBarrierGuard; -predicate indirectBarrierGuard(DataFlow::Node node, int indirectionIndex) { - node = BarrierGuard::getAnIndirectBarrierNode(indirectionIndex) +predicate indirectBarrierGuard(DataFlow::Node node, string s) { + node = BarrierGuard::getAnIndirectBarrierNode(_) and + if node.isGLValue() + then s = "glval<" + node.getType().toString().replaceAll(" ", "") + ">" + else s = node.getType().toString().replaceAll(" ", "") } -predicate barrierGuard(DataFlow::Node node) { node = BarrierGuard::getABarrierNode() } +predicate barrierGuard(DataFlow::Node node, string s) { + node = BarrierGuard::getABarrierNode() and + if node.isGLValue() + then s = "glval<" + node.getType().toString().replaceAll(" ", "") + ">" + else s = node.getType().toString().replaceAll(" ", "") +} module Test implements TestSig { - string getARelevantTag() { result = "barrier" } + string getARelevantTag() { result = ["barrier", "indirect_barrier"] } predicate hasActualResult(Location location, string element, string tag, string value) { - exists(DataFlow::Node node | - barrierGuard(node) and - value = "" + exists(DataFlow::Node node, string s | + indirectBarrierGuard(node, s) and + value = s and + tag = "indirect_barrier" or - exists(int indirectionIndex | - indirectBarrierGuard(node, indirectionIndex) and - value = indirectionIndex.toString() - ) + barrierGuard(node, s) and + value = s and + tag = "barrier" | - tag = "barrier" and element = node.toString() and location = node.getLocation() ) diff --git a/shared/controlflow/codeql/controlflow/Guards.qll b/shared/controlflow/codeql/controlflow/Guards.qll index b313afcdb6bb..f6774db1eb9f 100644 --- a/shared/controlflow/codeql/controlflow/Guards.qll +++ b/shared/controlflow/codeql/controlflow/Guards.qll @@ -1364,7 +1364,7 @@ module Make< /** * Holds if the guard `g` validates the expression `e` upon evaluating to `val`. */ - private predicate guardChecks(Guard g, Expr e, GuardValue val, P par) { + predicate guardChecks(Guard g, Expr e, GuardValue val, P par) { guardChecks0(g, e, val, par) or exists(NonOverridableMethodCall call, ParameterPosition ppos, ArgumentPosition apos |