From f19b381e3e199da88652958fd6fc05bc3b235f1f Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 8 Nov 2022 17:10:37 +0000 Subject: [PATCH 1/8] C++: Add use-use flow through global variables. --- .../ir/dataflow/internal/DataFlowPrivate.qll | 32 ++-- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 72 +++++++- .../cpp/ir/dataflow/internal/SsaInternals.qll | 163 +++++++++++++++++- .../dataflow-ir-consistency.expected | 15 -- .../dataflow/dataflow-tests/test.cpp | 2 +- .../fields/dataflow-ir-consistency.expected | 9 - .../dataflow/taint-tests/localTaint.expected | 55 ++++++ 7 files changed, 297 insertions(+), 51 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index ce9ba14ed026..cc7d7e667c55 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -345,21 +345,17 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) { */ predicate jumpStep(Node n1, Node n2) { exists(Cpp::GlobalOrNamespaceVariable v | - v = - n1.asInstruction() - .(StoreInstruction) - .getResultAddress() - .(VariableAddressInstruction) - .getAstVariable() and - v = n2.asVariable() + exists(Ssa::GlobalUse globalUse | + v = globalUse.getVariable() and + n1.(FinalGlobalValue).getGlobalUse() = globalUse and + v = n2.asVariable(globalUse.getIndirectionIndex()) + ) or - v = - n2.asInstruction() - .(LoadInstruction) - .getSourceAddress() - .(VariableAddressInstruction) - .getAstVariable() and - v = n1.asVariable() + exists(Ssa::GlobalDef globalDef | + v = globalDef.getVariable() and + v = n1.asVariable(globalDef.getIndirectionIndex()) and + n2.(InitialGlobalValue).getGlobalDef() = globalDef + ) ) } @@ -535,7 +531,13 @@ class Unit extends TUnit { } /** Holds if `n` should be hidden from path explanations. */ -predicate nodeIsHidden(Node n) { n instanceof OperandNode and not n instanceof ArgumentNode } +predicate nodeIsHidden(Node n) { + n instanceof OperandNode and not n instanceof ArgumentNode + or + n instanceof FinalGlobalValue + or + n instanceof InitialGlobalValue +} class LambdaCallKind = Unit; 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 12caadc552f2..fcd009f96ae4 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 @@ -42,7 +42,9 @@ private module Cached { (not i.getResultType() instanceof VoidType or i.isGLValue()) } or TOperandNode(Operand op) { not Ssa::ignoreOperand(op) } or - TVariableNode(Variable var) or + TVariableNode(Variable var, int indirectionIndex) { + indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())] + } or TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) { indirectionIndex = [1 .. Ssa::countIndirectionsForCppType(operand.getObjectAddress().getResultLanguageType())] @@ -57,7 +59,9 @@ private module Cached { } or TIndirectInstruction(Instruction instr, int indirectionIndex) { Ssa::hasIndirectInstruction(instr, indirectionIndex) - } + } or + TFinalGlobalValue(Ssa::GlobalUse globalUse) or + TInitialGlobalValue(Ssa::GlobalDef globalUse) } /** @@ -269,7 +273,14 @@ class Node extends TIRDataFlowNode { * Gets the variable corresponding to this node, if any. This can be used for * modeling flow in and out of global variables. */ - Variable asVariable() { result = this.(VariableNode).getVariable() } + Variable asVariable() { result = this.asVariable(1) } + + Variable asVariable(int indirectionIndex) { + exists(VariableNode varNode | this = varNode | + varNode.getVariable() = result and + varNode.getIndirectionIndex() = indirectionIndex + ) + } /** * Gets the expression that is partially defined by this node, if any. @@ -492,6 +503,42 @@ class SideEffectOperandNode extends Node, IndirectOperand { Expr getArgument() { result = call.getArgument(argumentIndex).getUnconvertedResultExpression() } } +class FinalGlobalValue extends Node, TFinalGlobalValue { + Ssa::GlobalUse globalUse; + + FinalGlobalValue() { this = TFinalGlobalValue(globalUse) } + + Ssa::GlobalUse getGlobalUse() { result = globalUse } + + override Declaration getEnclosingCallable() { result = this.getFunction() } + + override Declaration getFunction() { result = globalUse.getIRFunction().getFunction() } + + override DataFlowType getType() { result instanceof VoidType } // TODO + + final override Location getLocationImpl() { result = globalUse.getLocation() } + + override string toStringImpl() { result = "FinalGlobalValue" } +} + +class InitialGlobalValue extends Node, TInitialGlobalValue { + Ssa::GlobalDef globalDef; + + InitialGlobalValue() { this = TInitialGlobalValue(globalDef) } + + Ssa::GlobalDef getGlobalDef() { result = globalDef } + + override Declaration getEnclosingCallable() { result = this.getFunction() } + + override Declaration getFunction() { result = globalDef.getIRFunction().getFunction() } + + override DataFlowType getType() { result instanceof VoidType } // TODO + + final override Location getLocationImpl() { result = globalDef.getLocation() } + + override string toStringImpl() { result = "InitialGlobalValue" } +} + /** * INTERNAL: do not use. * @@ -1074,12 +1121,15 @@ class DefinitionByReferenceNode extends IndirectArgumentOutNode { */ class VariableNode extends Node, TVariableNode { Variable v; + int indirectionIndex; - VariableNode() { this = TVariableNode(v) } + VariableNode() { this = TVariableNode(v, indirectionIndex) } /** Gets the variable corresponding to this node. */ Variable getVariable() { result = v } + int getIndirectionIndex() { result = indirectionIndex } + override Declaration getFunction() { none() } override Declaration getEnclosingCallable() { @@ -1093,7 +1143,15 @@ class VariableNode extends Node, TVariableNode { override DataFlowType getType() { result = v.getType() } - final override Location getLocationImpl() { result = v.getLocation() } + final override Location getLocationImpl() { + // Certain variables (such as parameters) can have multiple locations. + // When there's a unique location we use that one, but if multiple locations + // exist we default to an unknown location. + result = unique( | | v.getLocation()) + or + not exists(unique( | | v.getLocation())) and + result instanceof UnknownDefaultLocation + } override string toStringImpl() { result = v.toString() } } @@ -1138,7 +1196,9 @@ DefinitionByReferenceNode definitionByReferenceNodeFromArgument(Expr argument) { } /** Gets the `VariableNode` corresponding to the variable `v`. */ -VariableNode variableNode(Variable v) { result.getVariable() = v } +VariableNode variableNode(Variable v) { + result.getVariable() = v and result.getIndirectionIndex() = 1 +} /** * DEPRECATED: See UninitializedNode. diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index e1ffd9af9a1d..10576fea26c8 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -107,14 +107,39 @@ predicate hasIndirectInstruction(Instruction instr, int indirectionIndex) { cached private newtype TDefOrUseImpl = TDefImpl(Operand address, int indirectionIndex) { - isDef(_, _, address, _, _, indirectionIndex) and - // We only include the definition if the SSA pruning stage - // concluded that the definition is live after the write. - any(SsaInternals0::Def def).getAddressOperand() = address + exists(Instruction base | isDef(_, _, address, base, _, indirectionIndex) | + // We only include the definition if the SSA pruning stage + // concluded that the definition is live after the write. + any(SsaInternals0::Def def).getAddressOperand() = address + or + // Since the pruning stage doesn't know about global variables we can't use the above check to + // rule out dead assignments to globals. + base.(VariableAddressInstruction).getAstVariable() instanceof Cpp::GlobalOrNamespaceVariable + ) } or TUseImpl(Operand operand, int indirectionIndex) { isUse(_, operand, _, _, indirectionIndex) and not isDef(_, _, operand, _, _, _) + } or + TGlobalUse(Cpp::GlobalOrNamespaceVariable v, IRFunction f, int indirectionIndex) { + // Represents a final "use" of a global variable to ensure that + // the assignment to a global variable isn't ruled out as dead. + exists(VariableAddressInstruction vai, int defIndex | + vai.getEnclosingIRFunction() = f and + vai.getAstVariable() = v and + isDef(_, _, _, vai, _, defIndex) and + indirectionIndex = [0 .. defIndex] + 1 + ) + } or + TGlobalDef(Cpp::GlobalOrNamespaceVariable v, IRFunction f, int indirectionIndex) { + // Represents the initial "definition" of a global variable when entering + // a function body. + exists(VariableAddressInstruction vai | + vai.getEnclosingIRFunction() = f and + vai.getAstVariable() = v and + isUse(_, _, vai, _, indirectionIndex) and + not isDef(_, _, vai.getAUse(), _, _, _) + ) } abstract private class DefOrUseImpl extends TDefOrUseImpl { @@ -254,6 +279,70 @@ class UseImpl extends DefOrUseImpl, TUseImpl { predicate isCertain() { isUse(true, operand, _, _, ind) } } +class GlobalUse extends TGlobalUse { + Cpp::GlobalOrNamespaceVariable global; + IRFunction f; + int indirectionIndex; + + GlobalUse() { this = TGlobalUse(global, f, indirectionIndex) } + + Cpp::GlobalOrNamespaceVariable getVariable() { result = global } + + IRFunction getIRFunction() { result = f } + + final predicate hasIndexInBlock(IRBlock block, int index) { + exists(ExitFunctionInstruction exit | + exit = f.getExitFunctionInstruction() and + block.getInstruction(index) = exit + ) + } + + int getIndirectionIndex() { result = indirectionIndex } + + SourceVariable getSourceVariable() { sourceVariableIsGlobal(result, global, f, indirectionIndex) } + + final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) { + this.hasIndexInBlock(block, index) and + sv = this.getSourceVariable() + } + + final Cpp::Location getLocation() { result = f.getLocation() } + + string toString() { result = global.toString() + " [final value from " + f.toString() + "]" } +} + +class GlobalDef extends TGlobalDef { + Cpp::GlobalOrNamespaceVariable global; + IRFunction f; + int indirectionIndex; + + GlobalDef() { this = TGlobalDef(global, f, indirectionIndex) } + + Cpp::GlobalOrNamespaceVariable getVariable() { result = global } + + IRFunction getIRFunction() { result = f } + + int getIndirectionIndex() { result = indirectionIndex } + + final predicate hasIndexInBlock(IRBlock block, int index) { + exists(EnterFunctionInstruction enter | + enter = f.getEnterFunctionInstruction() and + block.getInstruction(index) = enter + ) + } + + SourceVariable getSourceVariable() { sourceVariableIsGlobal(result, global, f, indirectionIndex) } + + final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) { + this.hasIndexInBlock(block, index) and + sv = this.getSourceVariable() + } + + final Cpp::Location getLocation() { result = f.getLocation() } + + string toString() { result = global.toString() + " [initial value in " + f.toString() + "]" } +} + /** * Holds if `defOrUse1` is a definition which is first read by `use`, * or if `defOrUse1` is a use and `use` is a next subsequent use. @@ -280,6 +369,32 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) { ) } +/** + * Holds if `defOrUse` should flow to the final use of the + * global variable use represetned by `globalUse`. + */ +private predicate defOrUseToGlobalUse(DefOrUse defOrUse, GlobalUse globalUse) { + exists(IRBlock bb1, int i1, IRBlock bb2, int i2, SourceVariable v | + defOrUse.asDefOrUse().hasIndexInBlock(bb1, i1, v) and + globalUse.hasIndexInBlock(bb2, i2, v) and + adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1), + pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) + ) +} + +/** + * Holds if `globalDef` represents the initial definition of a global variable that + * flows to `useOrPhi`. + */ +private predicate globalDefToUse(GlobalDef globalDef, UseOrPhi useOrPhi) { + exists(IRBlock bb1, int i1, IRBlock bb2, int i2, SourceVariable v | + globalDef.hasIndexInBlock(bb1, i1, v) and + adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1), + pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and + useOrPhi.asDefOrUse().hasIndexInBlock(bb2, i2, v) + ) +} + private predicate useToNode(UseOrPhi use, Node nodeTo) { exists(UseImpl useImpl | useImpl = use.asDefOrUse() and @@ -369,6 +484,20 @@ predicate ssaFlow(Node nodeFrom, Node nodeTo) { adjacentDefRead(defOrUse1, use) and useToNode(use, nodeTo) ) + or + // Def/use to final value of global vairable + exists(DefOrUse defOrUse, GlobalUse globalUse | + nodeToDefOrUse(nodeFrom, defOrUse) and + defOrUseToGlobalUse(defOrUse, globalUse) and + nodeTo.(FinalGlobalValue).getGlobalUse() = globalUse + ) + or + // Initial global variable value to a first use + exists(GlobalDef globalDef, UseOrPhi use | + nodeFrom.(InitialGlobalValue).getGlobalDef() = globalDef and + globalDefToUse(globalDef, use) and + useToNode(use, nodeTo) + ) } /** @@ -421,6 +550,17 @@ private predicate variableWriteCand(IRBlock bb, int i, SourceVariable v) { ) } +private predicate sourceVariableIsGlobal( + SourceVariable sv, Cpp::GlobalOrNamespaceVariable global, IRFunction func, int indirectionIndex +) { + exists(IRVariable irVar, BaseIRVariable base | + sourceVariableHasBaseAndIndex(sv, base, indirectionIndex) and + irVar = base.getIRVariable() and + irVar.getEnclosingIRFunction() = func and + global = irVar.getAst() + ) +} + private module SsaInput implements SsaImplCommon::InputSig { import InputSigCommon import SourceVariables @@ -431,10 +571,18 @@ private module SsaInput implements SsaImplCommon::InputSig { */ predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) { DataFlowImplCommon::forceCachingInSameStage() and - variableWriteCand(bb, i, v) and + ( + variableWriteCand(bb, i, v) or + sourceVariableIsGlobal(v, _, _, _) + ) and exists(DefImpl def | def.hasIndexInBlock(bb, i, v) | if def.isCertain() then certain = true else certain = false ) + or + exists(GlobalDef global | + global.hasIndexInBlock(bb, i, v) and + certain = true + ) } /** @@ -445,6 +593,11 @@ private module SsaInput implements SsaImplCommon::InputSig { exists(UseImpl use | use.hasIndexInBlock(bb, i, v) | if use.isCertain() then certain = true else certain = false ) + or + exists(GlobalUse global | + global.hasIndexInBlock(bb, i, v) and + certain = true + ) } } diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected index 7880941abd87..8a6e05cc613d 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected @@ -5,22 +5,7 @@ uniqueEnclosingCallable | globals.cpp:16:12:16:26 | VariableAddress indirection | Node should have one enclosing callable but has 0. | uniqueType uniqueNodeLocation -| BarrierGuard.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 6. | -| acrossLinkTargets.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 6. | -| clang.cpp:4:11:4:13 | (unnamed parameter 0) | Node should have one location but has 6. | -| clang.cpp:4:27:4:35 | (unnamed parameter 0) | Node should have one location but has 2. | -| clang.cpp:4:51:4:53 | (unnamed parameter 0) | Node should have one location but has 2. | -| dispatch.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 6. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| globals.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 6. | -| test.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 6. | -| test.cpp:2:27:2:35 | (unnamed parameter 0) | Node should have one location but has 2. | -| test.cpp:2:51:2:53 | (unnamed parameter 0) | Node should have one location but has 2. | missingLocation -| Nodes without location: 4 | uniqueNodeToString missingToString parameterCallable diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index 11c8b2a78f81..e2be2a87e1be 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -346,7 +346,7 @@ namespace FlowThroughGlobals { void taintAndCall() { globalVar = source(); calledAfterTaint(); - sink(globalVar); // $ ast ir ir=333:17 ir=347:17 + sink(globalVar); // $ ast ir=333:17 ir=347:17 } } diff --git a/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected index 3ee4797f0eee..55edc700c8f2 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected @@ -1,16 +1,7 @@ uniqueEnclosingCallable uniqueType uniqueNodeLocation -| E.cpp:15:31:15:33 | buf | Node should have one location but has 2. | -| aliasing.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 2. | -| conflated.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 2. | -| conflated.cpp:14:22:14:25 | buf | Node should have one location but has 2. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | -| file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. | missingLocation -| Nodes without location: 4 | uniqueNodeToString missingToString parameterCallable diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected index 1efc4d8fd353..bb5627376f94 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected @@ -80280,6 +80280,7 @@ | taint.cpp:25:5:25:11 | VariableAddress indirection | taint.cpp:25:5:25:11 | VariableAddress | TAINT | | taint.cpp:25:5:25:11 | VariableAddress indirection | taint.cpp:25:5:25:11 | VariableAddress indirection | | | taint.cpp:25:15:25:15 | 0 | taint.cpp:25:15:25:15 | StoreValue | | +| taint.cpp:25:15:25:15 | Store | taint.cpp:25:5:25:11 | FinalGlobalValue | | | taint.cpp:25:15:25:15 | StoreValue | taint.cpp:25:15:25:15 | Store | | | taint.cpp:26:5:26:11 | VariableAddress | taint.cpp:26:5:26:11 | Address | | | taint.cpp:26:5:26:11 | VariableAddress indirection | taint.cpp:26:5:26:11 | Address | TAINT | @@ -80290,7 +80291,10 @@ | taint.cpp:26:15:26:20 | FunctionAddress indirection | taint.cpp:26:15:26:20 | CallTarget | TAINT | | taint.cpp:26:15:26:20 | FunctionAddress indirection | taint.cpp:26:15:26:20 | FunctionAddress | TAINT | | taint.cpp:26:15:26:20 | FunctionAddress indirection | taint.cpp:26:15:26:20 | FunctionAddress indirection | | +| taint.cpp:26:15:26:20 | Store | taint.cpp:26:5:26:11 | FinalGlobalValue | | | taint.cpp:26:15:26:20 | call to source | taint.cpp:26:15:26:20 | Store | | +| taint.cpp:27:5:27:11 | InitialGlobalValue | taint.cpp:27:15:27:21 | Address | | +| taint.cpp:27:5:27:11 | InitialGlobalValue | taint.cpp:27:15:27:21 | global2 | | | taint.cpp:27:5:27:11 | VariableAddress | taint.cpp:27:5:27:11 | Address | | | taint.cpp:27:5:27:11 | VariableAddress indirection | taint.cpp:27:5:27:11 | Address | TAINT | | taint.cpp:27:5:27:11 | VariableAddress indirection | taint.cpp:27:5:27:11 | VariableAddress | TAINT | @@ -80300,10 +80304,12 @@ | taint.cpp:27:15:27:21 | Load | taint.cpp:27:15:27:21 | Left | | | taint.cpp:27:15:27:21 | VariableAddress | taint.cpp:27:15:27:21 | Address | | | taint.cpp:27:15:27:21 | global2 | taint.cpp:27:15:27:21 | Address | TAINT | +| taint.cpp:27:15:27:21 | global2 | taint.cpp:27:15:27:21 | Left | | | taint.cpp:27:15:27:21 | global2 | taint.cpp:27:15:27:21 | Load | | | taint.cpp:27:15:27:21 | global2 indirection | taint.cpp:27:15:27:21 | VariableAddress | TAINT | | taint.cpp:27:15:27:21 | global2 indirection | taint.cpp:27:15:27:21 | global2 | | | taint.cpp:27:15:27:25 | ... + ... | taint.cpp:27:15:27:25 | StoreValue | | +| taint.cpp:27:15:27:25 | Store | taint.cpp:27:5:27:11 | FinalGlobalValue | | | taint.cpp:27:15:27:25 | StoreValue | taint.cpp:27:15:27:25 | Store | | | taint.cpp:27:25:27:25 | 1 | taint.cpp:27:25:27:25 | Right | | | taint.cpp:27:25:27:25 | Right | taint.cpp:27:15:27:25 | ... + ... | TAINT | @@ -80316,6 +80322,7 @@ | taint.cpp:28:15:28:23 | FunctionAddress indirection | taint.cpp:28:15:28:23 | CallTarget | TAINT | | taint.cpp:28:15:28:23 | FunctionAddress indirection | taint.cpp:28:15:28:23 | FunctionAddress | TAINT | | taint.cpp:28:15:28:23 | FunctionAddress indirection | taint.cpp:28:15:28:23 | FunctionAddress indirection | | +| taint.cpp:28:15:28:23 | Store | taint.cpp:28:5:28:11 | FinalGlobalValue | | | taint.cpp:28:15:28:23 | call to increment | taint.cpp:28:15:28:23 | Store | | | taint.cpp:28:25:28:30 | Call | taint.cpp:28:25:28:30 | call to source | | | taint.cpp:28:25:28:30 | FunctionAddress | taint.cpp:28:25:28:30 | CallTarget | | @@ -80331,12 +80338,23 @@ | taint.cpp:29:15:29:18 | FunctionAddress indirection | taint.cpp:29:15:29:18 | CallTarget | TAINT | | taint.cpp:29:15:29:18 | FunctionAddress indirection | taint.cpp:29:15:29:18 | FunctionAddress | TAINT | | taint.cpp:29:15:29:18 | FunctionAddress indirection | taint.cpp:29:15:29:18 | FunctionAddress indirection | | +| taint.cpp:29:15:29:18 | Store | taint.cpp:29:5:29:11 | FinalGlobalValue | | | taint.cpp:29:15:29:18 | call to zero | taint.cpp:29:15:29:18 | Store | | | taint.cpp:29:20:29:25 | Call | taint.cpp:29:20:29:25 | call to source | | | taint.cpp:29:20:29:25 | FunctionAddress | taint.cpp:29:20:29:25 | CallTarget | | | taint.cpp:29:20:29:25 | FunctionAddress indirection | taint.cpp:29:20:29:25 | CallTarget | TAINT | | taint.cpp:29:20:29:25 | FunctionAddress indirection | taint.cpp:29:20:29:25 | FunctionAddress | TAINT | | taint.cpp:29:20:29:25 | FunctionAddress indirection | taint.cpp:29:20:29:25 | FunctionAddress indirection | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:36:12:36:18 | Address | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:36:12:36:18 | global7 | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:40:7:40:13 | Address | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:40:7:40:13 | global6 indirection | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:42:7:42:13 | Address | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:42:7:42:13 | global8 indirection | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:43:7:43:13 | Address | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:43:7:43:13 | global9 indirection | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:44:7:44:14 | Address | | +| taint.cpp:32:6:32:14 | InitialGlobalValue | taint.cpp:44:7:44:14 | global10 indirection | | | taint.cpp:34:2:34:8 | global6 | taint.cpp:34:2:34:8 | Address | | | taint.cpp:34:2:34:8 | global6 indirection | taint.cpp:34:2:34:8 | Address | TAINT | | taint.cpp:34:2:34:8 | global6 indirection | taint.cpp:34:2:34:8 | global6 | TAINT | @@ -80361,6 +80379,7 @@ | taint.cpp:36:2:36:8 | global8 indirection | taint.cpp:36:2:36:8 | global8 indirection | | | taint.cpp:36:2:36:22 | Store | taint.cpp:42:7:42:13 | global8 indirection | | | taint.cpp:36:12:36:18 | Address | taint.cpp:36:12:36:18 | Load | TAINT | +| taint.cpp:36:12:36:18 | Address | taint.cpp:41:7:41:13 | Address | | | taint.cpp:36:12:36:18 | Left | taint.cpp:36:12:36:22 | ... + ... | TAINT | | taint.cpp:36:12:36:18 | Left | taint.cpp:41:7:41:13 | global7 indirection | | | taint.cpp:36:12:36:18 | Load | taint.cpp:36:12:36:18 | Left | | @@ -80368,6 +80387,7 @@ | taint.cpp:36:12:36:18 | global7 | taint.cpp:36:12:36:18 | Address | TAINT | | taint.cpp:36:12:36:18 | global7 | taint.cpp:36:12:36:18 | Left | | | taint.cpp:36:12:36:18 | global7 | taint.cpp:36:12:36:18 | Load | | +| taint.cpp:36:12:36:18 | global7 | taint.cpp:41:7:41:13 | global7 indirection | | | taint.cpp:36:12:36:18 | global7 indirection | taint.cpp:36:12:36:18 | VariableAddress | TAINT | | taint.cpp:36:12:36:18 | global7 indirection | taint.cpp:36:12:36:18 | global7 | | | taint.cpp:36:12:36:22 | ... + ... | taint.cpp:36:12:36:22 | StoreValue | | @@ -80415,6 +80435,7 @@ | taint.cpp:40:7:40:13 | VariableAddress | taint.cpp:40:7:40:13 | Address | | | taint.cpp:40:7:40:13 | VariableAddress indirection | taint.cpp:40:7:40:13 | VariableAddress | TAINT | | taint.cpp:40:7:40:13 | VariableAddress indirection | taint.cpp:40:7:40:13 | global6 indirection | | +| taint.cpp:40:7:40:13 | global6 | taint.cpp:32:6:32:14 | FinalGlobalValue | | | taint.cpp:40:7:40:13 | global6 indirection | taint.cpp:40:7:40:13 | Address | TAINT | | taint.cpp:40:7:40:13 | global6 indirection | taint.cpp:40:7:40:13 | Load | | | taint.cpp:40:7:40:13 | global6 indirection | taint.cpp:40:7:40:13 | global6 | | @@ -80427,6 +80448,7 @@ | taint.cpp:41:7:41:13 | VariableAddress | taint.cpp:41:7:41:13 | Address | | | taint.cpp:41:7:41:13 | VariableAddress indirection | taint.cpp:41:7:41:13 | VariableAddress | TAINT | | taint.cpp:41:7:41:13 | VariableAddress indirection | taint.cpp:41:7:41:13 | global7 indirection | | +| taint.cpp:41:7:41:13 | global7 | taint.cpp:32:6:32:14 | FinalGlobalValue | | | taint.cpp:41:7:41:13 | global7 indirection | taint.cpp:41:7:41:13 | Address | TAINT | | taint.cpp:41:7:41:13 | global7 indirection | taint.cpp:41:7:41:13 | Load | | | taint.cpp:41:7:41:13 | global7 indirection | taint.cpp:41:7:41:13 | global7 | | @@ -80439,6 +80461,7 @@ | taint.cpp:42:7:42:13 | VariableAddress | taint.cpp:42:7:42:13 | Address | | | taint.cpp:42:7:42:13 | VariableAddress indirection | taint.cpp:42:7:42:13 | VariableAddress | TAINT | | taint.cpp:42:7:42:13 | VariableAddress indirection | taint.cpp:42:7:42:13 | global8 indirection | | +| taint.cpp:42:7:42:13 | global8 | taint.cpp:32:6:32:14 | FinalGlobalValue | | | taint.cpp:42:7:42:13 | global8 indirection | taint.cpp:42:7:42:13 | Address | TAINT | | taint.cpp:42:7:42:13 | global8 indirection | taint.cpp:42:7:42:13 | Load | | | taint.cpp:42:7:42:13 | global8 indirection | taint.cpp:42:7:42:13 | global8 | | @@ -80451,6 +80474,7 @@ | taint.cpp:43:7:43:13 | VariableAddress | taint.cpp:43:7:43:13 | Address | | | taint.cpp:43:7:43:13 | VariableAddress indirection | taint.cpp:43:7:43:13 | VariableAddress | TAINT | | taint.cpp:43:7:43:13 | VariableAddress indirection | taint.cpp:43:7:43:13 | global9 indirection | | +| taint.cpp:43:7:43:13 | global9 | taint.cpp:32:6:32:14 | FinalGlobalValue | | | taint.cpp:43:7:43:13 | global9 indirection | taint.cpp:43:7:43:13 | Address | TAINT | | taint.cpp:43:7:43:13 | global9 indirection | taint.cpp:43:7:43:13 | Load | | | taint.cpp:43:7:43:13 | global9 indirection | taint.cpp:43:7:43:13 | global9 | | @@ -80463,9 +80487,30 @@ | taint.cpp:44:7:44:14 | VariableAddress | taint.cpp:44:7:44:14 | Address | | | taint.cpp:44:7:44:14 | VariableAddress indirection | taint.cpp:44:7:44:14 | VariableAddress | TAINT | | taint.cpp:44:7:44:14 | VariableAddress indirection | taint.cpp:44:7:44:14 | global10 indirection | | +| taint.cpp:44:7:44:14 | global10 | taint.cpp:32:6:32:14 | FinalGlobalValue | | | taint.cpp:44:7:44:14 | global10 indirection | taint.cpp:44:7:44:14 | Address | TAINT | | taint.cpp:44:7:44:14 | global10 indirection | taint.cpp:44:7:44:14 | Load | | | taint.cpp:44:7:44:14 | global10 indirection | taint.cpp:44:7:44:14 | global10 | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:49:7:49:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:49:7:49:13 | global1 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:50:7:50:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:50:7:50:13 | global2 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:51:7:51:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:51:7:51:13 | global3 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:52:7:52:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:52:7:52:13 | global4 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:53:7:53:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:53:7:53:13 | global5 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:54:7:54:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:54:7:54:13 | global6 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:55:7:55:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:55:7:55:13 | global7 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:56:7:56:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:56:7:56:13 | global8 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:57:7:57:13 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:57:7:57:13 | global9 indirection | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:58:7:58:14 | Address | | +| taint.cpp:47:6:47:12 | InitialGlobalValue | taint.cpp:58:7:58:14 | global10 indirection | | | taint.cpp:49:2:49:5 | FunctionAddress | taint.cpp:49:2:49:5 | CallTarget | | | taint.cpp:49:2:49:5 | FunctionAddress indirection | taint.cpp:49:2:49:5 | CallTarget | TAINT | | taint.cpp:49:2:49:5 | FunctionAddress indirection | taint.cpp:49:2:49:5 | FunctionAddress | TAINT | @@ -80477,6 +80522,7 @@ | taint.cpp:49:7:49:13 | VariableAddress indirection | taint.cpp:49:7:49:13 | global1 indirection | | | taint.cpp:49:7:49:13 | global1 indirection | taint.cpp:49:7:49:13 | Address | TAINT | | taint.cpp:49:7:49:13 | global1 indirection | taint.cpp:49:7:49:13 | Load | | +| taint.cpp:49:7:49:13 | global1 indirection | taint.cpp:49:7:49:13 | global1 | | | taint.cpp:50:2:50:5 | FunctionAddress | taint.cpp:50:2:50:5 | CallTarget | | | taint.cpp:50:2:50:5 | FunctionAddress indirection | taint.cpp:50:2:50:5 | CallTarget | TAINT | | taint.cpp:50:2:50:5 | FunctionAddress indirection | taint.cpp:50:2:50:5 | FunctionAddress | TAINT | @@ -80488,6 +80534,7 @@ | taint.cpp:50:7:50:13 | VariableAddress indirection | taint.cpp:50:7:50:13 | global2 indirection | | | taint.cpp:50:7:50:13 | global2 indirection | taint.cpp:50:7:50:13 | Address | TAINT | | taint.cpp:50:7:50:13 | global2 indirection | taint.cpp:50:7:50:13 | Load | | +| taint.cpp:50:7:50:13 | global2 indirection | taint.cpp:50:7:50:13 | global2 | | | taint.cpp:51:2:51:5 | FunctionAddress | taint.cpp:51:2:51:5 | CallTarget | | | taint.cpp:51:2:51:5 | FunctionAddress indirection | taint.cpp:51:2:51:5 | CallTarget | TAINT | | taint.cpp:51:2:51:5 | FunctionAddress indirection | taint.cpp:51:2:51:5 | FunctionAddress | TAINT | @@ -80499,6 +80546,7 @@ | taint.cpp:51:7:51:13 | VariableAddress indirection | taint.cpp:51:7:51:13 | global3 indirection | | | taint.cpp:51:7:51:13 | global3 indirection | taint.cpp:51:7:51:13 | Address | TAINT | | taint.cpp:51:7:51:13 | global3 indirection | taint.cpp:51:7:51:13 | Load | | +| taint.cpp:51:7:51:13 | global3 indirection | taint.cpp:51:7:51:13 | global3 | | | taint.cpp:52:2:52:5 | FunctionAddress | taint.cpp:52:2:52:5 | CallTarget | | | taint.cpp:52:2:52:5 | FunctionAddress indirection | taint.cpp:52:2:52:5 | CallTarget | TAINT | | taint.cpp:52:2:52:5 | FunctionAddress indirection | taint.cpp:52:2:52:5 | FunctionAddress | TAINT | @@ -80510,6 +80558,7 @@ | taint.cpp:52:7:52:13 | VariableAddress indirection | taint.cpp:52:7:52:13 | global4 indirection | | | taint.cpp:52:7:52:13 | global4 indirection | taint.cpp:52:7:52:13 | Address | TAINT | | taint.cpp:52:7:52:13 | global4 indirection | taint.cpp:52:7:52:13 | Load | | +| taint.cpp:52:7:52:13 | global4 indirection | taint.cpp:52:7:52:13 | global4 | | | taint.cpp:53:2:53:5 | FunctionAddress | taint.cpp:53:2:53:5 | CallTarget | | | taint.cpp:53:2:53:5 | FunctionAddress indirection | taint.cpp:53:2:53:5 | CallTarget | TAINT | | taint.cpp:53:2:53:5 | FunctionAddress indirection | taint.cpp:53:2:53:5 | FunctionAddress | TAINT | @@ -80521,6 +80570,7 @@ | taint.cpp:53:7:53:13 | VariableAddress indirection | taint.cpp:53:7:53:13 | global5 indirection | | | taint.cpp:53:7:53:13 | global5 indirection | taint.cpp:53:7:53:13 | Address | TAINT | | taint.cpp:53:7:53:13 | global5 indirection | taint.cpp:53:7:53:13 | Load | | +| taint.cpp:53:7:53:13 | global5 indirection | taint.cpp:53:7:53:13 | global5 | | | taint.cpp:54:2:54:5 | FunctionAddress | taint.cpp:54:2:54:5 | CallTarget | | | taint.cpp:54:2:54:5 | FunctionAddress indirection | taint.cpp:54:2:54:5 | CallTarget | TAINT | | taint.cpp:54:2:54:5 | FunctionAddress indirection | taint.cpp:54:2:54:5 | FunctionAddress | TAINT | @@ -80532,6 +80582,7 @@ | taint.cpp:54:7:54:13 | VariableAddress indirection | taint.cpp:54:7:54:13 | global6 indirection | | | taint.cpp:54:7:54:13 | global6 indirection | taint.cpp:54:7:54:13 | Address | TAINT | | taint.cpp:54:7:54:13 | global6 indirection | taint.cpp:54:7:54:13 | Load | | +| taint.cpp:54:7:54:13 | global6 indirection | taint.cpp:54:7:54:13 | global6 | | | taint.cpp:55:2:55:5 | FunctionAddress | taint.cpp:55:2:55:5 | CallTarget | | | taint.cpp:55:2:55:5 | FunctionAddress indirection | taint.cpp:55:2:55:5 | CallTarget | TAINT | | taint.cpp:55:2:55:5 | FunctionAddress indirection | taint.cpp:55:2:55:5 | FunctionAddress | TAINT | @@ -80543,6 +80594,7 @@ | taint.cpp:55:7:55:13 | VariableAddress indirection | taint.cpp:55:7:55:13 | global7 indirection | | | taint.cpp:55:7:55:13 | global7 indirection | taint.cpp:55:7:55:13 | Address | TAINT | | taint.cpp:55:7:55:13 | global7 indirection | taint.cpp:55:7:55:13 | Load | | +| taint.cpp:55:7:55:13 | global7 indirection | taint.cpp:55:7:55:13 | global7 | | | taint.cpp:56:2:56:5 | FunctionAddress | taint.cpp:56:2:56:5 | CallTarget | | | taint.cpp:56:2:56:5 | FunctionAddress indirection | taint.cpp:56:2:56:5 | CallTarget | TAINT | | taint.cpp:56:2:56:5 | FunctionAddress indirection | taint.cpp:56:2:56:5 | FunctionAddress | TAINT | @@ -80554,6 +80606,7 @@ | taint.cpp:56:7:56:13 | VariableAddress indirection | taint.cpp:56:7:56:13 | global8 indirection | | | taint.cpp:56:7:56:13 | global8 indirection | taint.cpp:56:7:56:13 | Address | TAINT | | taint.cpp:56:7:56:13 | global8 indirection | taint.cpp:56:7:56:13 | Load | | +| taint.cpp:56:7:56:13 | global8 indirection | taint.cpp:56:7:56:13 | global8 | | | taint.cpp:57:2:57:5 | FunctionAddress | taint.cpp:57:2:57:5 | CallTarget | | | taint.cpp:57:2:57:5 | FunctionAddress indirection | taint.cpp:57:2:57:5 | CallTarget | TAINT | | taint.cpp:57:2:57:5 | FunctionAddress indirection | taint.cpp:57:2:57:5 | FunctionAddress | TAINT | @@ -80565,6 +80618,7 @@ | taint.cpp:57:7:57:13 | VariableAddress indirection | taint.cpp:57:7:57:13 | global9 indirection | | | taint.cpp:57:7:57:13 | global9 indirection | taint.cpp:57:7:57:13 | Address | TAINT | | taint.cpp:57:7:57:13 | global9 indirection | taint.cpp:57:7:57:13 | Load | | +| taint.cpp:57:7:57:13 | global9 indirection | taint.cpp:57:7:57:13 | global9 | | | taint.cpp:58:2:58:5 | FunctionAddress | taint.cpp:58:2:58:5 | CallTarget | | | taint.cpp:58:2:58:5 | FunctionAddress indirection | taint.cpp:58:2:58:5 | CallTarget | TAINT | | taint.cpp:58:2:58:5 | FunctionAddress indirection | taint.cpp:58:2:58:5 | FunctionAddress | TAINT | @@ -80576,6 +80630,7 @@ | taint.cpp:58:7:58:14 | VariableAddress indirection | taint.cpp:58:7:58:14 | global10 indirection | | | taint.cpp:58:7:58:14 | global10 indirection | taint.cpp:58:7:58:14 | Address | TAINT | | taint.cpp:58:7:58:14 | global10 indirection | taint.cpp:58:7:58:14 | Load | | +| taint.cpp:58:7:58:14 | global10 indirection | taint.cpp:58:7:58:14 | global10 | | | taint.cpp:63:2:63:10 | FunctionAddress | taint.cpp:63:2:63:10 | CallTarget | | | taint.cpp:63:2:63:10 | FunctionAddress indirection | taint.cpp:63:2:63:10 | CallTarget | TAINT | | taint.cpp:63:2:63:10 | FunctionAddress indirection | taint.cpp:63:2:63:10 | FunctionAddress | TAINT | From 8c224429b3657605240d0eb988c22b4c2e657dd5 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 13 Jan 2023 17:12:30 +0000 Subject: [PATCH 2/8] C++: Better 'getType' for global variable nodes. --- .../ir/dataflow/internal/DataFlowPrivate.qll | 14 ++++-- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 44 ++++++++++++++----- .../cpp/ir/dataflow/internal/SsaInternals.qll | 16 ++++++- .../Likely Bugs/Format/NonConstantFormat.ql | 3 ++ 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 81421c1c84e3..2e9ffbc2961f 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -516,14 +516,22 @@ predicate jumpStep(Node n1, Node n2) { exists(Cpp::GlobalOrNamespaceVariable v | exists(Ssa::GlobalUse globalUse | v = globalUse.getVariable() and - n1.(FinalGlobalValue).getGlobalUse() = globalUse and - v = n2.asVariable(globalUse.getIndirectionIndex()) + n1.(FinalGlobalValue).getGlobalUse() = globalUse + | + globalUse.getIndirectionIndex() = 1 and + v = n2.asVariable() + or + v = n2.asIndirectVariable(globalUse.getIndirectionIndex()) ) or exists(Ssa::GlobalDef globalDef | v = globalDef.getVariable() and - v = n1.asVariable(globalDef.getIndirectionIndex()) and n2.(InitialGlobalValue).getGlobalDef() = globalDef + | + globalDef.getIndirectionIndex() = 1 and + v = n1.asVariable() + or + v = n1.asIndirectVariable(globalDef.getIndirectionIndex()) ) ) } 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 4ac253b039d4..226d00fff8aa 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 @@ -268,15 +268,21 @@ class Node extends TIRDataFlowNode { * Gets the variable corresponding to this node, if any. This can be used for * modeling flow in and out of global variables. */ - Variable asVariable() { result = this.asVariable(1) } + Variable asVariable() { this = TVariableNode(result, 1) } - Variable asVariable(int indirectionIndex) { - exists(VariableNode varNode | this = varNode | - varNode.getVariable() = result and - varNode.getIndirectionIndex() = indirectionIndex - ) + /** + * Gets the `indirectionIndex`'th indirection of this node's underlying variable, if any. + * + * This can be used for modeling flow in and out of global variables. + */ + Variable asIndirectVariable(int indirectionIndex) { + indirectionIndex > 1 and + this = TVariableNode(result, indirectionIndex) } + /** Gets an indirection of this node's underlying variable, if any. */ + Variable asIndirectVariable() { result = this.asIndirectVariable(_) } + /** * Gets the expression that is partially defined by this node, if any. * @@ -510,11 +516,16 @@ class FinalGlobalValue extends Node, TFinalGlobalValue { override Declaration getFunction() { result = globalUse.getIRFunction().getFunction() } - override DataFlowType getType() { result instanceof VoidType } // TODO + override DataFlowType getType() { + exists(int indirectionIndex | + indirectionIndex = globalUse.getIndirectionIndex() and + result = getTypeImpl(globalUse.getUnspecifiedType(), indirectionIndex - 1) + ) + } final override Location getLocationImpl() { result = globalUse.getLocation() } - override string toStringImpl() { result = "FinalGlobalValue" } + override string toStringImpl() { result = globalUse.toString() } } class InitialGlobalValue extends Node, TInitialGlobalValue { @@ -528,11 +539,16 @@ class InitialGlobalValue extends Node, TInitialGlobalValue { override Declaration getFunction() { result = globalDef.getIRFunction().getFunction() } - override DataFlowType getType() { result instanceof VoidType } // TODO + override DataFlowType getType() { + exists(int indirectionIndex | + indirectionIndex = globalDef.getIndirectionIndex() and + result = getTypeImpl(globalDef.getUnspecifiedType(), indirectionIndex) + ) + } final override Location getLocationImpl() { result = globalDef.getLocation() } - override string toStringImpl() { result = "InitialGlobalValue" } + override string toStringImpl() { result = globalDef.toString() } } /** @@ -1173,7 +1189,9 @@ class VariableNode extends Node, TVariableNode { result = v } - override DataFlowType getType() { result = v.getType() } + override DataFlowType getType() { + result = getTypeImpl(v.getUnspecifiedType(), indirectionIndex - 1) + } final override Location getLocationImpl() { // Certain variables (such as parameters) can have multiple locations. @@ -1185,7 +1203,9 @@ class VariableNode extends Node, TVariableNode { result instanceof UnknownDefaultLocation } - override string toStringImpl() { result = v.toString() } + override string toStringImpl() { + if indirectionIndex = 1 then result = v.toString() else result = v.toString() + " indirection" + } } /** diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 4fb500b72f92..507bfd061cb3 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -359,7 +359,13 @@ class GlobalUse extends TGlobalUse { final Cpp::Location getLocation() { result = f.getLocation() } - string toString() { result = global.toString() + " [final value from " + f.toString() + "]" } + string toString() { + if indirectionIndex = 1 + then result = global.toString() + else result = global.toString() + " indirection" + } + + Type getUnspecifiedType() { result = global.getUnspecifiedType() } } class GlobalDef extends TGlobalDef { @@ -391,7 +397,13 @@ class GlobalDef extends TGlobalDef { final Cpp::Location getLocation() { result = f.getLocation() } - string toString() { result = global.toString() + " [initial value in " + f.toString() + "]" } + string toString() { + if indirectionIndex = 0 + then result = global.toString() + else result = global.toString() + " indirection" + } + + Type getUnspecifiedType() { result = global.getUnspecifiedType() } } /** diff --git a/cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql b/cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql index ffb85602a308..7f2cd96d6b22 100644 --- a/cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql +++ b/cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql @@ -122,6 +122,9 @@ predicate isSanitizerNode(DataFlow::Node node) { or not exists(node.asIndirectExpr()) and not exists(node.asDefiningArgument()) and + not exists(node.asIndirectVariable()) and + not node instanceof DataFlow::InitialGlobalValue and + not node instanceof DataFlow::FinalGlobalValue and cannotContainString(node.getType(), false) } From ee62f2a2233e83df08be4427f9502606a9a2057b Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 16 Jan 2023 15:21:16 +0000 Subject: [PATCH 3/8] C++: Fix global variable exclusion in DTT. --- .../code/cpp/ir/dataflow/internal/DefaultTaintTrackingImpl.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DefaultTaintTrackingImpl.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DefaultTaintTrackingImpl.qll index b007d1a3a6fa..0e2fe9b22203 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DefaultTaintTrackingImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DefaultTaintTrackingImpl.qll @@ -622,6 +622,8 @@ module TaintedWithPath { private predicate isGlobalVariablePathNode(WrapPathNode n) { n.inner().getNode().asVariable() instanceof GlobalOrNamespaceVariable + or + n.inner().getNode().asIndirectVariable() instanceof GlobalOrNamespaceVariable } private predicate edgesWithoutGlobals(PathNode a, PathNode b) { From 6a91e859819c67494454a991121442eed9e3b0f6 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 26 Jan 2023 16:01:37 -0500 Subject: [PATCH 4/8] C++: fix UseImpl after merge conflict --- .../semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 72cb78401406..39bba28a3e6b 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -339,6 +339,10 @@ abstract private class OperandBasedUse extends UseImpl { operand.getUse() = block.getInstruction(index) } + final Operand getOperand() { + result = operand + } + final override Cpp::Location getLocation() { result = operand.getLocation() } } @@ -538,7 +542,7 @@ private predicate globalDefToUse(GlobalDef globalDef, UseOrPhi useOrPhi) { } private predicate useToNode(UseOrPhi use, Node nodeTo) { - exists(UseImpl useImpl | + exists(OperandBasedUse useImpl | useImpl = use.asDefOrUse() and nodeHasOperand(nodeTo, useImpl.getOperand(), useImpl.getIndirectionIndex()) ) From 136b5d189c7f23fdd3b473cd4d4eb27612e64332 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 1 Feb 2023 13:24:40 +0000 Subject: [PATCH 5/8] C++: Small cleanup by making 'GlobalUse' extend 'UseImpl'. --- .../cpp/ir/dataflow/internal/SsaInternals.qll | 78 ++++++------------- 1 file changed, 24 insertions(+), 54 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 39bba28a3e6b..4240de951276 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -221,7 +221,7 @@ abstract private class DefOrUseImpl extends TDefOrUseImpl { } /** Gets the variable that is defined or used. */ - final SourceVariable getSourceVariable() { + SourceVariable getSourceVariable() { exists(BaseSourceVariable v, int ind | sourceVariableHasBaseAndIndex(result, v, ind) and defOrUseHasSourceVariable(this, v, ind) @@ -339,9 +339,7 @@ abstract private class OperandBasedUse extends UseImpl { operand.getUse() = block.getInstruction(index) } - final Operand getOperand() { - result = operand - } + final Operand getOperand() { result = operand } final override Cpp::Location getLocation() { result = operand.getLocation() } } @@ -372,6 +370,14 @@ private class IteratorUse extends OperandBasedUse, TIteratorUse { override Node getNode() { nodeHasOperand(result, operand, ind) } } +pragma[nomagic] +private predicate finalParameterNodeHasParameterAndIndex( + FinalParameterNode n, Parameter p, int indirectionIndex +) { + n.getParameter() = p and + n.getIndirectionIndex() = indirectionIndex +} + class FinalParameterUse extends UseImpl, TFinalParameterUse { Parameter p; @@ -379,10 +385,7 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse { Parameter getParameter() { result = p } - override Node getNode() { - result.(FinalParameterNode).getParameter() = p and - result.(FinalParameterNode).getIndirectionIndex() = ind - } + override Node getNode() { finalParameterNodeHasParameterAndIndex(result, p, ind) } override int getIndirection() { result = ind + 1 } @@ -413,42 +416,36 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse { } } -class GlobalUse extends TGlobalUse { +class GlobalUse extends UseImpl, TGlobalUse { Cpp::GlobalOrNamespaceVariable global; IRFunction f; - int indirectionIndex; - GlobalUse() { this = TGlobalUse(global, f, indirectionIndex) } + GlobalUse() { this = TGlobalUse(global, f, ind) } + + override FinalGlobalValue getNode() { result.getGlobalUse() = this } + + override int getIndirection() { result = ind + 1 } // TODO Cpp::GlobalOrNamespaceVariable getVariable() { result = global } IRFunction getIRFunction() { result = f } - final predicate hasIndexInBlock(IRBlock block, int index) { + final override predicate hasIndexInBlock(IRBlock block, int index) { exists(ExitFunctionInstruction exit | exit = f.getExitFunctionInstruction() and block.getInstruction(index) = exit ) } - int getIndirectionIndex() { result = indirectionIndex } + override SourceVariable getSourceVariable() { sourceVariableIsGlobal(result, global, f, ind) } - SourceVariable getSourceVariable() { sourceVariableIsGlobal(result, global, f, indirectionIndex) } + final override Cpp::Location getLocation() { result = f.getLocation() } - final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) { - this.hasIndexInBlock(block, index) and - sv = this.getSourceVariable() - } - - final Cpp::Location getLocation() { result = f.getLocation() } + Type getUnspecifiedType() { result = global.getUnspecifiedType() } - string toString() { - if indirectionIndex = 1 - then result = global.toString() - else result = global.toString() + " indirection" - } + override predicate isCertain() { any() } - Type getUnspecifiedType() { result = global.getUnspecifiedType() } + override BaseSourceVariableInstruction getBase() { none() } } class GlobalDef extends TGlobalDef { @@ -515,19 +512,6 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) { ) } -/** - * Holds if `defOrUse` should flow to the final use of the - * global variable use represetned by `globalUse`. - */ -private predicate defOrUseToGlobalUse(DefOrUse defOrUse, GlobalUse globalUse) { - exists(IRBlock bb1, int i1, IRBlock bb2, int i2, SourceVariable v | - defOrUse.asDefOrUse().hasIndexInBlock(bb1, i1, v) and - globalUse.hasIndexInBlock(bb2, i2, v) and - adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1), - pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) - ) -} - /** * Holds if `globalDef` represents the initial definition of a global variable that * flows to `useOrPhi`. @@ -541,14 +525,7 @@ private predicate globalDefToUse(GlobalDef globalDef, UseOrPhi useOrPhi) { ) } -private predicate useToNode(UseOrPhi use, Node nodeTo) { - exists(OperandBasedUse useImpl | - useImpl = use.asDefOrUse() and - nodeHasOperand(nodeTo, useImpl.getOperand(), useImpl.getIndirectionIndex()) - ) - or - nodeTo.(SsaPhiNode).getPhiNode() = use.asPhi() -} +private predicate useToNode(UseOrPhi use, Node nodeTo) { use.getNode() = nodeTo } pragma[noinline] predicate outNodeHasAddressAndIndex( @@ -636,13 +613,6 @@ private predicate ssaFlowImpl(Node nodeFrom, Node nodeTo, boolean uncertain) { useToNode(use, nodeTo) ) or - // Def/use to final value of global vairable - exists(DefOrUse defOrUse, GlobalUse globalUse | - nodeToDefOrUse(nodeFrom, defOrUse, uncertain) and - defOrUseToGlobalUse(defOrUse, globalUse) and - nodeTo.(FinalGlobalValue).getGlobalUse() = globalUse - ) - or // Initial global variable value to a first use exists(GlobalDef globalDef, UseOrPhi use | nodeFrom.(InitialGlobalValue).getGlobalDef() = globalDef and From 0e1dcc8062591119bc547a449c8b419574fbd52c Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 1 Feb 2023 13:25:37 +0000 Subject: [PATCH 6/8] C++: Accept test changes. These all appear to be good changes. --- .../semmle/consts/NonConstantFormat.expected | 1 + .../UncontrolledFormatString.expected | 36 ++++++------------- ...olledFormatStringThroughGlobalVar.expected | 36 +++++++++++++++++++ .../CWE/CWE-319/UseOfHttp/UseOfHttp.expected | 8 ++--- .../semmle/tests/ExposedSystemData.expected | 4 --- .../PotentiallyExposedSystemData.expected | 8 +---- .../Security/CWE/CWE-611/XXE.expected | 6 ++++ 7 files changed, 56 insertions(+), 43 deletions(-) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected index 8f63ffb72d86..6a4d7c8d7a99 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected @@ -8,6 +8,7 @@ | consts.cpp:112:9:112:10 | v6 | The format string argument to printf should be constant to prevent security issues and other potential errors. | | consts.cpp:116:9:116:13 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. | | consts.cpp:121:9:121:10 | v8 | The format string argument to printf should be constant to prevent security issues and other potential errors. | +| consts.cpp:126:9:126:27 | call to nonConstFuncToArray | The format string argument to printf should be constant to prevent security issues and other potential errors. | | consts.cpp:130:9:130:10 | v9 | The format string argument to printf should be constant to prevent security issues and other potential errors. | | consts.cpp:135:9:135:11 | v10 | The format string argument to printf should be constant to prevent security issues and other potential errors. | | consts.cpp:140:9:140:11 | v11 | The format string argument to printf should be constant to prevent security issues and other potential errors. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatString.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatString.expected index a01a3f4b5cbb..33015407b831 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatString.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatString.expected @@ -1,61 +1,45 @@ edges | globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | | globalVars.c:8:7:8:10 | copy | globalVars.c:35:11:35:14 | copy | | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | -| globalVars.c:11:22:11:25 | argv | globalVars.c:12:2:12:15 | ... = ... | -| globalVars.c:12:2:12:15 | ... = ... | globalVars.c:8:7:8:10 | copy | -| globalVars.c:15:21:15:23 | val | globalVars.c:16:2:16:12 | ... = ... | -| globalVars.c:16:2:16:12 | ... = ... | globalVars.c:9:7:9:11 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:11:22:11:25 | argv | globalVars.c:8:7:8:10 | copy | +| globalVars.c:15:21:15:23 | val | globalVars.c:9:7:9:11 | copy2 | | globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv | | globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv | -| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | -| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | -| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | -| globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy | -| globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy | | globalVars.c:35:11:35:14 | copy | globalVars.c:15:21:15:23 | val | -| globalVars.c:35:11:35:14 | copy | globalVars.c:35:11:35:14 | copy | -| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | -| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | -| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | -| globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 | -| globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 | -| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | -| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | -| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | subpaths nodes | globalVars.c:8:7:8:10 | copy | semmle.label | copy | | globalVars.c:9:7:9:11 | copy2 | semmle.label | copy2 | | globalVars.c:11:22:11:25 | argv | semmle.label | argv | -| globalVars.c:12:2:12:15 | ... = ... | semmle.label | ... = ... | | globalVars.c:15:21:15:23 | val | semmle.label | val | -| globalVars.c:16:2:16:12 | ... = ... | semmle.label | ... = ... | | globalVars.c:24:11:24:14 | argv | semmle.label | argv | | globalVars.c:24:11:24:14 | argv | semmle.label | argv | | globalVars.c:27:9:27:12 | copy | semmle.label | copy | | globalVars.c:27:9:27:12 | copy | semmle.label | copy | | globalVars.c:27:9:27:12 | copy | semmle.label | copy | -| globalVars.c:27:9:27:12 | copy | semmle.label | copy | | globalVars.c:30:15:30:18 | copy | semmle.label | copy | | globalVars.c:30:15:30:18 | copy | semmle.label | copy | -| globalVars.c:30:15:30:18 | copy | semmle.label | copy | -| globalVars.c:35:11:35:14 | copy | semmle.label | copy | | globalVars.c:35:11:35:14 | copy | semmle.label | copy | | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | -| globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | -| globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | -| globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | #select | globalVars.c:27:9:27:12 | copy | globalVars.c:24:11:24:14 | argv | globalVars.c:27:9:27:12 | copy | The value of this argument may come from $@ and is being used as a formatting argument to printf(format). | globalVars.c:24:11:24:14 | argv | argv | | globalVars.c:30:15:30:18 | copy | globalVars.c:24:11:24:14 | argv | globalVars.c:30:15:30:18 | copy | The value of this argument may come from $@ and is being used as a formatting argument to printWrapper(str), which calls printf(format). | globalVars.c:24:11:24:14 | argv | argv | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected index a01a3f4b5cbb..8ac81fd9e6d4 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected @@ -1,12 +1,30 @@ edges | globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:33:15:33:18 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:35:11:35:14 | copy | | globalVars.c:8:7:8:10 | copy | globalVars.c:35:11:35:14 | copy | | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:44:15:44:19 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:11:22:11:25 | argv | globalVars.c:8:7:8:10 | copy | | globalVars.c:11:22:11:25 | argv | globalVars.c:12:2:12:15 | ... = ... | | globalVars.c:12:2:12:15 | ... = ... | globalVars.c:8:7:8:10 | copy | +| globalVars.c:15:21:15:23 | val | globalVars.c:9:7:9:11 | copy2 | | globalVars.c:15:21:15:23 | val | globalVars.c:16:2:16:12 | ... = ... | | globalVars.c:16:2:16:12 | ... = ... | globalVars.c:9:7:9:11 | copy2 | | globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv | @@ -14,15 +32,31 @@ edges | globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | | globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | | globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:27:9:27:12 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:27:9:27:12 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:27:9:27:12 | copy | globalVars.c:35:11:35:14 | copy | | globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy | | globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:30:15:30:18 | copy | globalVars.c:35:11:35:14 | copy | +| globalVars.c:33:15:33:18 | copy | globalVars.c:35:11:35:14 | copy | | globalVars.c:35:11:35:14 | copy | globalVars.c:15:21:15:23 | val | | globalVars.c:35:11:35:14 | copy | globalVars.c:35:11:35:14 | copy | | globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | | globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | | globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | | globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 | | globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:41:15:41:19 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:41:15:41:19 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:41:15:41:19 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:44:15:44:19 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:44:15:44:19 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:44:15:44:19 | copy2 | globalVars.c:50:9:50:13 | copy2 | | globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | | globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | | globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | @@ -43,6 +77,7 @@ nodes | globalVars.c:30:15:30:18 | copy | semmle.label | copy | | globalVars.c:30:15:30:18 | copy | semmle.label | copy | | globalVars.c:30:15:30:18 | copy | semmle.label | copy | +| globalVars.c:33:15:33:18 | copy | semmle.label | copy | | globalVars.c:35:11:35:14 | copy | semmle.label | copy | | globalVars.c:35:11:35:14 | copy | semmle.label | copy | | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | @@ -52,6 +87,7 @@ nodes | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | +| globalVars.c:44:15:44:19 | copy2 | semmle.label | copy2 | | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected index 6d5cc34e0751..2ebb9d168e45 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected @@ -2,9 +2,8 @@ edges | test.cpp:11:26:11:28 | url | test.cpp:15:30:15:32 | url | | test.cpp:11:26:11:28 | url indirection | test.cpp:15:30:15:32 | url | | test.cpp:24:13:24:17 | url_g | test.cpp:38:11:38:15 | url_g | -| test.cpp:24:21:24:40 | array to pointer conversion | test.cpp:24:13:24:17 | url_g | -| test.cpp:24:21:24:40 | http://example.com | test.cpp:24:21:24:40 | array to pointer conversion | -| test.cpp:24:21:24:40 | http://example.com | test.cpp:24:21:24:40 | array to pointer conversion | +| test.cpp:24:21:24:40 | http://example.com | test.cpp:24:13:24:17 | url_g | +| test.cpp:24:21:24:40 | http://example.com | test.cpp:24:13:24:17 | url_g | | test.cpp:28:10:28:29 | http://example.com | test.cpp:11:26:11:28 | url | | test.cpp:28:10:28:29 | http://example.com | test.cpp:28:10:28:29 | http://example.com | | test.cpp:35:23:35:42 | http://example.com | test.cpp:39:11:39:15 | url_l | @@ -14,7 +13,6 @@ edges | test.cpp:36:26:36:45 | http://example.com | test.cpp:40:11:40:17 | access to array indirection | | test.cpp:36:26:36:45 | http://example.com | test.cpp:40:11:40:17 | access to array indirection | | test.cpp:38:11:38:15 | url_g | test.cpp:11:26:11:28 | url | -| test.cpp:38:11:38:15 | url_g | test.cpp:38:11:38:15 | url_g | | test.cpp:39:11:39:15 | url_l | test.cpp:11:26:11:28 | url | | test.cpp:40:11:40:17 | access to array | test.cpp:11:26:11:28 | url | | test.cpp:40:11:40:17 | access to array indirection | test.cpp:11:26:11:28 | url indirection | @@ -37,7 +35,6 @@ nodes | test.cpp:11:26:11:28 | url indirection | semmle.label | url indirection | | test.cpp:15:30:15:32 | url | semmle.label | url | | test.cpp:24:13:24:17 | url_g | semmle.label | url_g | -| test.cpp:24:21:24:40 | array to pointer conversion | semmle.label | array to pointer conversion | | test.cpp:24:21:24:40 | http://example.com | semmle.label | http://example.com | | test.cpp:24:21:24:40 | http://example.com | semmle.label | http://example.com | | test.cpp:28:10:28:29 | http://example.com | semmle.label | http://example.com | @@ -47,7 +44,6 @@ nodes | test.cpp:36:26:36:45 | http://example.com | semmle.label | http://example.com | | test.cpp:36:26:36:45 | http://example.com | semmle.label | http://example.com | | test.cpp:38:11:38:15 | url_g | semmle.label | url_g | -| test.cpp:38:11:38:15 | url_g | semmle.label | url_g | | test.cpp:39:11:39:15 | url_l | semmle.label | url_l | | test.cpp:40:11:40:17 | access to array | semmle.label | access to array | | test.cpp:40:11:40:17 | access to array indirection | semmle.label | access to array indirection | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected index 09bc08470d4f..025d4937fa81 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected @@ -1,7 +1,6 @@ edges | tests2.cpp:50:13:50:19 | global1 | tests2.cpp:82:14:82:20 | global1 | | tests2.cpp:50:23:50:43 | call to mysql_get_client_info | tests2.cpp:50:13:50:19 | global1 | -| tests2.cpp:50:23:50:43 | call to mysql_get_client_info | tests2.cpp:50:23:50:43 | call to mysql_get_client_info | | tests2.cpp:63:13:63:18 | call to getenv | tests2.cpp:63:13:63:26 | call to getenv | | tests2.cpp:64:13:64:18 | call to getenv | tests2.cpp:64:13:64:26 | call to getenv | | tests2.cpp:65:13:65:18 | call to getenv | tests2.cpp:65:13:65:30 | call to getenv | @@ -9,7 +8,6 @@ edges | tests2.cpp:78:18:78:38 | call to mysql_get_client_info | tests2.cpp:81:14:81:19 | buffer | | tests2.cpp:78:18:78:38 | call to mysql_get_client_info | tests2.cpp:81:14:81:19 | buffer | | tests2.cpp:78:18:78:38 | call to mysql_get_client_info | tests2.cpp:81:14:81:19 | buffer | -| tests2.cpp:82:14:82:20 | global1 | tests2.cpp:82:14:82:20 | global1 | | tests2.cpp:91:42:91:45 | str1 | tests2.cpp:93:14:93:17 | str1 | | tests2.cpp:101:8:101:15 | call to getpwuid | tests2.cpp:102:14:102:15 | pw | | tests2.cpp:109:3:109:36 | ... = ... | tests2.cpp:109:6:109:8 | c1 indirection [post update] [ptr] | @@ -32,7 +30,6 @@ edges nodes | tests2.cpp:50:13:50:19 | global1 | semmle.label | global1 | | tests2.cpp:50:23:50:43 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info | -| tests2.cpp:50:23:50:43 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info | | tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv | | tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv | | tests2.cpp:63:13:63:26 | call to getenv | semmle.label | call to getenv | @@ -51,7 +48,6 @@ nodes | tests2.cpp:81:14:81:19 | buffer | semmle.label | buffer | | tests2.cpp:81:14:81:19 | buffer | semmle.label | buffer | | tests2.cpp:82:14:82:20 | global1 | semmle.label | global1 | -| tests2.cpp:82:14:82:20 | global1 | semmle.label | global1 | | tests2.cpp:91:42:91:45 | str1 | semmle.label | str1 | | tests2.cpp:93:14:93:17 | str1 | semmle.label | str1 | | tests2.cpp:101:8:101:15 | call to getpwuid | semmle.label | call to getpwuid | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected index ab8e7ec0eb44..3a1595a4e6a0 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/PotentiallyExposedSystemData.expected @@ -5,12 +5,9 @@ edges | tests.cpp:57:18:57:23 | call to getenv | tests.cpp:57:18:57:39 | call to getenv | | tests.cpp:58:41:58:46 | call to getenv | tests.cpp:58:41:58:62 | call to getenv | | tests.cpp:59:43:59:48 | call to getenv | tests.cpp:59:43:59:64 | call to getenv | -| tests.cpp:62:7:62:18 | global_token | tests.cpp:69:17:69:28 | global_token | | tests.cpp:62:7:62:18 | global_token | tests.cpp:71:27:71:38 | global_token | +| tests.cpp:62:7:62:18 | global_token | tests.cpp:73:27:73:31 | maybe | | tests.cpp:62:22:62:27 | call to getenv | tests.cpp:62:7:62:18 | global_token | -| tests.cpp:62:22:62:27 | call to getenv | tests.cpp:62:22:62:27 | call to getenv | -| tests.cpp:69:17:69:28 | global_token | tests.cpp:73:27:73:31 | maybe | -| tests.cpp:71:27:71:38 | global_token | tests.cpp:71:27:71:38 | global_token | | tests.cpp:86:29:86:31 | msg | tests.cpp:88:15:88:17 | msg | | tests.cpp:97:13:97:18 | call to getenv | tests.cpp:97:13:97:34 | call to getenv | | tests.cpp:97:13:97:18 | call to getenv | tests.cpp:97:13:97:34 | call to getenv | @@ -50,9 +47,6 @@ nodes | tests.cpp:59:43:59:64 | call to getenv | semmle.label | call to getenv | | tests.cpp:62:7:62:18 | global_token | semmle.label | global_token | | tests.cpp:62:22:62:27 | call to getenv | semmle.label | call to getenv | -| tests.cpp:62:22:62:27 | call to getenv | semmle.label | call to getenv | -| tests.cpp:69:17:69:28 | global_token | semmle.label | global_token | -| tests.cpp:71:27:71:38 | global_token | semmle.label | global_token | | tests.cpp:71:27:71:38 | global_token | semmle.label | global_token | | tests.cpp:73:27:73:31 | maybe | semmle.label | maybe | | tests.cpp:86:29:86:31 | msg | semmle.label | msg | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected index cacfd2102e34..632bc727ae44 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected @@ -8,6 +8,8 @@ edges | tests5.cpp:27:25:27:38 | call to createLSParser indirection | tests5.cpp:29:2:29:2 | p indirection | | tests5.cpp:40:25:40:38 | call to createLSParser indirection | tests5.cpp:43:2:43:2 | p indirection | | tests5.cpp:55:25:55:38 | call to createLSParser indirection | tests5.cpp:59:2:59:2 | p indirection | +| tests5.cpp:63:21:63:24 | g_p2 indirection | tests5.cpp:77:2:77:5 | g_p2 indirection | +| tests5.cpp:70:17:70:30 | call to createLSParser indirection | tests5.cpp:63:21:63:24 | g_p2 indirection | | tests5.cpp:81:25:81:38 | call to createLSParser indirection | tests5.cpp:83:2:83:2 | p indirection | | tests5.cpp:81:25:81:38 | call to createLSParser indirection | tests5.cpp:83:2:83:2 | p indirection | | tests5.cpp:83:2:83:2 | p indirection | tests5.cpp:85:2:85:2 | p indirection | @@ -65,6 +67,9 @@ nodes | tests5.cpp:43:2:43:2 | p indirection | semmle.label | p indirection | | tests5.cpp:55:25:55:38 | call to createLSParser indirection | semmle.label | call to createLSParser indirection | | tests5.cpp:59:2:59:2 | p indirection | semmle.label | p indirection | +| tests5.cpp:63:21:63:24 | g_p2 indirection | semmle.label | g_p2 indirection | +| tests5.cpp:70:17:70:30 | call to createLSParser indirection | semmle.label | call to createLSParser indirection | +| tests5.cpp:77:2:77:5 | g_p2 indirection | semmle.label | g_p2 indirection | | tests5.cpp:81:25:81:38 | call to createLSParser indirection | semmle.label | call to createLSParser indirection | | tests5.cpp:83:2:83:2 | p indirection | semmle.label | p indirection | | tests5.cpp:83:2:83:2 | p indirection | semmle.label | p indirection | @@ -124,6 +129,7 @@ subpaths | tests5.cpp:29:2:29:2 | p indirection | tests5.cpp:27:25:27:38 | call to createLSParser indirection | tests5.cpp:29:2:29:2 | p indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:27:25:27:38 | call to createLSParser indirection | XML parser | | tests5.cpp:43:2:43:2 | p indirection | tests5.cpp:40:25:40:38 | call to createLSParser indirection | tests5.cpp:43:2:43:2 | p indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:40:25:40:38 | call to createLSParser indirection | XML parser | | tests5.cpp:59:2:59:2 | p indirection | tests5.cpp:55:25:55:38 | call to createLSParser indirection | tests5.cpp:59:2:59:2 | p indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:55:25:55:38 | call to createLSParser indirection | XML parser | +| tests5.cpp:77:2:77:5 | g_p2 indirection | tests5.cpp:70:17:70:30 | call to createLSParser indirection | tests5.cpp:77:2:77:5 | g_p2 indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:70:17:70:30 | call to createLSParser indirection | XML parser | | tests5.cpp:83:2:83:2 | p indirection | tests5.cpp:81:25:81:38 | call to createLSParser indirection | tests5.cpp:83:2:83:2 | p indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser indirection | XML parser | | tests5.cpp:89:2:89:2 | p indirection | tests5.cpp:81:25:81:38 | call to createLSParser indirection | tests5.cpp:89:2:89:2 | p indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser indirection | XML parser | | tests.cpp:17:2:17:2 | p indirection | tests.cpp:15:23:15:43 | call to XercesDOMParser | tests.cpp:17:2:17:2 | p indirection | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:15:23:15:43 | call to XercesDOMParser | XML parser | From b53963a791756b960b3f9ca7a738edd50eacfa56 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 2 Feb 2023 09:09:20 +0000 Subject: [PATCH 7/8] C++: QLDoc. --- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 15 +++++++ .../cpp/ir/dataflow/internal/SsaInternals.qll | 40 ++++++++++++++++++- .../dataflow/internal/SsaInternalsCommon.qll | 3 ++ 3 files changed, 57 insertions(+), 1 deletion(-) 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 0e7372a3bc3e..7ee36571586d 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 @@ -526,11 +526,18 @@ class SideEffectOperandNode extends Node, IndirectOperand { Expr getArgument() { result = call.getArgument(argumentIndex).getUnconvertedResultExpression() } } +/** + * INTERNAL: do not use. + * + * A node representing the value of a global variable just before returning + * from a function body. + */ class FinalGlobalValue extends Node, TFinalGlobalValue { Ssa::GlobalUse globalUse; FinalGlobalValue() { this = TFinalGlobalValue(globalUse) } + /** Gets the underlying SSA use. */ Ssa::GlobalUse getGlobalUse() { result = globalUse } override Declaration getEnclosingCallable() { result = this.getFunction() } @@ -549,11 +556,18 @@ class FinalGlobalValue extends Node, TFinalGlobalValue { override string toStringImpl() { result = globalUse.toString() } } +/** + * INTERNAL: do not use. + * + * A node representing the value of a global variable just after entering + * a function body. + */ class InitialGlobalValue extends Node, TInitialGlobalValue { Ssa::GlobalDef globalDef; InitialGlobalValue() { this = TInitialGlobalValue(globalDef) } + /** Gets the underlying SSA definition. */ Ssa::GlobalDef getGlobalDef() { result = globalDef } override Declaration getEnclosingCallable() { result = this.getFunction() } @@ -1262,6 +1276,7 @@ class VariableNode extends Node, TVariableNode { /** Gets the variable corresponding to this node. */ Variable getVariable() { result = v } + /** Gets the indirection index of this node. */ int getIndirectionIndex() { result = indirectionIndex } override Declaration getFunction() { none() } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index a5796a2c4ae8..bf9ce406249d 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -34,10 +34,19 @@ private module SourceVariables { bindingset[ind] SourceVariable() { any() } + /** Gets a textual representation of this element. */ abstract string toString(); + /** + * Gets the number of loads performed on the base source variable + * to reach the value of this source variable. + */ int getIndirection() { result = ind } + /** + * Gets the base source variable (i.e., the variable without any + * indirections) of this source variable. + */ abstract BaseSourceVariable getBaseVariable(); } @@ -192,6 +201,10 @@ abstract private class DefOrUseImpl extends TDefOrUseImpl { /** Holds if this definition or use has index `index` in block `block`. */ abstract predicate hasIndexInBlock(IRBlock block, int index); + /** + * Holds if this definition (or use) has index `index` in block `block`, + * and is a definition (or use) of the variable `sv` + */ final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) { this.hasIndexInBlock(block, index) and sv = this.getSourceVariable() @@ -216,6 +229,10 @@ abstract private class DefOrUseImpl extends TDefOrUseImpl { */ abstract BaseSourceVariableInstruction getBase(); + /** + * Gets the base source variable (i.e., the variable without + * any indirection) of this definition or use. + */ final BaseSourceVariable getBaseSourceVariable() { this.getBase().getBaseSourceVariable() = result } @@ -437,10 +454,12 @@ class GlobalUse extends UseImpl, TGlobalUse { override FinalGlobalValue getNode() { result.getGlobalUse() = this } - override int getIndirection() { result = ind + 1 } // TODO + override int getIndirection() { result = ind + 1 } + /** Gets the global variable associated with this use. */ Cpp::GlobalOrNamespaceVariable getVariable() { result = global } + /** Gets the `IRFunction` whose body is exited from after this use. */ IRFunction getIRFunction() { result = f } final override predicate hasIndexInBlock(IRBlock block, int index) { @@ -454,6 +473,10 @@ class GlobalUse extends UseImpl, TGlobalUse { final override Cpp::Location getLocation() { result = f.getLocation() } + /** + * Gets the type of this use after specifiers have been deeply stripped + * and typedefs have been resolved. + */ Type getUnspecifiedType() { result = global.getUnspecifiedType() } override predicate isCertain() { any() } @@ -468,12 +491,16 @@ class GlobalDef extends TGlobalDef { GlobalDef() { this = TGlobalDef(global, f, indirectionIndex) } + /** Gets the global variable associated with this definition. */ Cpp::GlobalOrNamespaceVariable getVariable() { result = global } + /** Gets the `IRFunction` whose body is evaluated after this definition. */ IRFunction getIRFunction() { result = f } + /** Gets the global variable associated with this definition. */ int getIndirectionIndex() { result = indirectionIndex } + /** Holds if this definition or use has index `index` in block `block`. */ final predicate hasIndexInBlock(IRBlock block, int index) { exists(EnterFunctionInstruction enter | enter = f.getEnterFunctionInstruction() and @@ -481,21 +508,32 @@ class GlobalDef extends TGlobalDef { ) } + /** Gets the global variable associated with this definition. */ SourceVariable getSourceVariable() { sourceVariableIsGlobal(result, global, f, indirectionIndex) } + /** + * Holds if this definition has index `index` in block `block`, and + * is a definition of the variable `sv` + */ final predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) { this.hasIndexInBlock(block, index) and sv = this.getSourceVariable() } + /** Gets the location of this element. */ final Cpp::Location getLocation() { result = f.getLocation() } + /** Gets a textual representation of this element. */ string toString() { if indirectionIndex = 0 then result = global.toString() else result = global.toString() + " indirection" } + /** + * Gets the type of this use after specifiers have been deeply stripped + * and typedefs have been resolved. + */ Type getUnspecifiedType() { result = global.getUnspecifiedType() } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll index 1649185309d8..d848525d99c1 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll @@ -325,8 +325,10 @@ newtype TBaseSourceVariable = TBaseCallVariable(AllocationInstruction call) abstract class BaseSourceVariable extends TBaseSourceVariable { + /** Gets a textual representation of this element. */ abstract string toString(); + /** Gets the type of this base source variable. */ abstract DataFlowType getType(); } @@ -441,6 +443,7 @@ predicate isModifiableAt(CppType cppType, int indirectionIndex) { } abstract class BaseSourceVariableInstruction extends Instruction { + /** Gets the base source variable accessed by this instruction. */ abstract BaseSourceVariable getBaseSourceVariable(); } From ae774a6b95e48bd0cac0f332dcb073a5a2344383 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 3 Feb 2023 16:59:54 +0000 Subject: [PATCH 8/8] C++: Add a test with an indirect source. --- .../dataflow-consistency.expected | 2 ++ .../dataflow/dataflow-tests/test.cpp | 26 +++++++++++++++++++ .../dataflow/dataflow-tests/test.ql | 4 +++ 3 files changed, 32 insertions(+) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected index fb5f66883119..025801f6b40c 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected @@ -105,6 +105,8 @@ postWithInFlow | test.cpp:542:6:542:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. | | test.cpp:548:25:548:25 | x [inner post update] | PostUpdateNode should not be the target of local flow. | | test.cpp:552:25:552:25 | y [inner post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:562:5:562:13 | globalInt [post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:576:5:576:13 | globalInt [post update] | PostUpdateNode should not be the target of local flow. | viableImplInCallContextTooLarge uniqueParameterNodeAtPosition uniqueParameterNodePosition diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index f2be988548b7..39f0488de2b4 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -552,3 +552,29 @@ void test_sink_then_source() { sink_then_source_2(&y, y); } } + +int* indirect_source(); + +namespace IndirectFlowThroughGlobals { + int* globalInt; + + void taintGlobal() { + globalInt = indirect_source(); + } + + void f() { + sink(*globalInt); // $ ir=562:17 ir=576:17 // tainted or clean? Not sure. + taintGlobal(); + sink(*globalInt); // $ ir=562:17 MISSING: ast=562:17 SPURIOUS: ir=576:17 + } + + void calledAfterTaint() { + sink(*globalInt); // $ ir=576:17 MISSING: ast=576:17 SPURIOUS: ir=562:17 + } + + void taintAndCall() { + globalInt = indirect_source(); + calledAfterTaint(); + sink(*globalInt); // $ ir=576:17 MISSING: ast=576:17 SPURIOUS: ir=562:17 + } +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.ql b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.ql index 82128fe371c4..2b7927a3a154 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.ql +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.ql @@ -24,6 +24,8 @@ module AstTest { or source.asParameter().getName().matches("source%") or + source.asExpr().(FunctionCall).getTarget().getName() = "indirect_source" + or source.(DataFlow::DefinitionByReferenceNode).getParameter().getName().matches("ref_source%") or // Track uninitialized variables @@ -67,6 +69,8 @@ module IRTest { override predicate isSource(DataFlow::Node source) { source.asExpr().(FunctionCall).getTarget().getName() = "source" or + source.asIndirectExpr(1).(FunctionCall).getTarget().getName() = "indirect_source" + or source.asParameter().getName().matches("source%") or source.(DataFlow::DefinitionByReferenceNode).getParameter().getName().matches("ref_source%")