diff --git a/change-notes/1.24/analysis-cpp.md b/change-notes/1.24/analysis-cpp.md new file mode 100644 index 00000000000..2e7fdd422af --- /dev/null +++ b/change-notes/1.24/analysis-cpp.md @@ -0,0 +1,20 @@ +# Improvements to C/C++ analysis + +The following changes in version 1.24 affect C/C++ analysis in all applications. + +## General improvements + +## New queries + +| **Query** | **Tags** | **Purpose** | +|-----------------------------|-----------|--------------------------------------------------------------------| + +## Changes to existing queries + +| **Query** | **Expected impact** | **Change** | +|----------------------------|------------------------|------------------------------------------------------------------| +| No space for zero terminator (`cpp/no-space-for-terminator`) | Fewer false positive results | This query has been modified to be more conservative when identifying which pointers point to null-terminated strings. This approach produces fewer, more accurate results. | + +## Changes to libraries + +* diff --git a/change-notes/1.24/analysis-javascript.md b/change-notes/1.24/analysis-javascript.md index 53400f537ce..fbe7fd00899 100644 --- a/change-notes/1.24/analysis-javascript.md +++ b/change-notes/1.24/analysis-javascript.md @@ -4,6 +4,7 @@ * Support for the following frameworks and libraries has been improved: - [react](https://www.npmjs.com/package/react) + - [Handlebars](https://www.npmjs.com/package/handlebars) ## New queries diff --git a/config/identical-files.json b/config/identical-files.json index 4fdac5c7fea..29601d19510 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -143,6 +143,11 @@ "cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll", "csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll" ], + "IR SSASanity": [ + "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll", + "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll", + "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.qll" + ], "C++ IR InstructionImports": [ "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionImports.qll", "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/InstructionImports.qll", diff --git a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql index ff17daca05c..5ed0f1bd679 100644 --- a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql +++ b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql @@ -16,28 +16,28 @@ import cpp import semmle.code.cpp.dataflow.DataFlow -import semmle.code.cpp.models.implementations.Memcpy +import semmle.code.cpp.models.interfaces.ArrayFunction class MallocCall extends FunctionCall { MallocCall() { this.getTarget().hasGlobalOrStdName("malloc") } - Expr getAllocatedSize() { - if this.getArgument(0) instanceof VariableAccess - then - exists(LocalScopeVariable v, ControlFlowNode def | - definitionUsePair(v, def, this.getArgument(0)) and - exprDefinition(v, def, result) - ) - else result = this.getArgument(0) - } + Expr getAllocatedSize() { result = this.getArgument(0) } } predicate terminationProblem(MallocCall malloc, string msg) { - malloc.getAllocatedSize() instanceof StrlenCall and - not exists(FunctionCall fc, MemcpyFunction memcpy, int ix | - DataFlow::localExprFlow(malloc, fc.getArgument(ix)) and - fc.getTarget() = memcpy and - memcpy.hasArrayOutput(ix) + // malloc(strlen(...)) + exists(StrlenCall strlen | DataFlow::localExprFlow(strlen, malloc.getAllocatedSize())) and + // flows into a null-terminated string function + exists(ArrayFunction af, FunctionCall fc, int arg | + DataFlow::localExprFlow(malloc, fc.getArgument(arg)) and + fc.getTarget() = af and + ( + // null terminated string + af.hasArrayWithNullTerminator(arg) + or + // likely a null terminated string (such as `strcpy`, `strcat`) + af.hasArrayWithUnknownSize(arg) + ) ) and msg = "This allocation does not include space to null-terminate the string." } diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll index 8b9c8f5abd3..91f5aeacc9d 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll @@ -57,14 +57,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -95,7 +95,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -183,7 +183,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -191,7 +191,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -226,7 +226,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -260,7 +260,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlowNoCtx(p, mid) and localValueStep(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -296,8 +296,8 @@ private module ImplCommon { setterCall(call, i1, i2, f) and node1.(ArgumentNode).argumentOf(call, i1) and node2.getPreUpdateNode().(ArgumentNode).argumentOf(call, i2) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -333,8 +333,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | storeReturn0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -365,8 +365,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | read0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getContainerType()) and - compatibleTypes(node2.getTypeBound(), f.getType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getContainerType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getType()) ) } @@ -384,7 +384,7 @@ private module ImplCommon { store(node1, f, mid1) and localValueStep*(mid1, mid2) and read(mid2, f, node2) and - compatibleTypes(node1.getTypeBound(), node2.getTypeBound()) + compatibleTypes(getErasedNodeTypeBound(node1), getErasedNodeTypeBound(node2)) ) } @@ -405,14 +405,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -443,7 +443,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -531,7 +531,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -539,7 +539,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -574,7 +574,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -860,4 +860,10 @@ private module ImplCommon { or result = viableCallable(call) and cc instanceof CallContextReturn } + + pragma[noinline] + DataFlowType getErasedNodeType(Node n) { result = getErasedRepr(n.getType()) } + + pragma[noinline] + DataFlowType getErasedNodeTypeBound(Node n) { result = getErasedRepr(n.getTypeBound()) } } diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll index 7ecb474f632..9519a74265f 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll index 8b9c8f5abd3..91f5aeacc9d 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll @@ -57,14 +57,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -95,7 +95,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -183,7 +183,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -191,7 +191,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -226,7 +226,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -260,7 +260,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlowNoCtx(p, mid) and localValueStep(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -296,8 +296,8 @@ private module ImplCommon { setterCall(call, i1, i2, f) and node1.(ArgumentNode).argumentOf(call, i1) and node2.getPreUpdateNode().(ArgumentNode).argumentOf(call, i2) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -333,8 +333,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | storeReturn0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -365,8 +365,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | read0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getContainerType()) and - compatibleTypes(node2.getTypeBound(), f.getType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getContainerType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getType()) ) } @@ -384,7 +384,7 @@ private module ImplCommon { store(node1, f, mid1) and localValueStep*(mid1, mid2) and read(mid2, f, node2) and - compatibleTypes(node1.getTypeBound(), node2.getTypeBound()) + compatibleTypes(getErasedNodeTypeBound(node1), getErasedNodeTypeBound(node2)) ) } @@ -405,14 +405,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -443,7 +443,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -531,7 +531,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -539,7 +539,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -574,7 +574,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -860,4 +860,10 @@ private module ImplCommon { or result = viableCallable(call) and cc instanceof CallContextReturn } + + pragma[noinline] + DataFlowType getErasedNodeType(Node n) { result = getErasedRepr(n.getType()) } + + pragma[noinline] + DataFlowType getErasedNodeTypeBound(Node n) { result = getErasedRepr(n.getTypeBound()) } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll index e10e266b2ab..f9efad42fb0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll @@ -13,14 +13,20 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var } /** - * Represents a variable referenced by the IR for a function. The variable may - * be a user-declared variable (`IRUserVariable`) or a temporary variable - * generated by the AST-to-IR translation (`IRTempVariable`). + * A variable referenced by the IR for a function. The variable may be a user-declared variable + * (`IRUserVariable`) or a temporary variable generated by the AST-to-IR translation + * (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + IRVariable() { + this = TIRUserVariable(_, _, func) or + this = TIRTempVariable(func, _, _, _) or + this = TIRStringLiteral(func, _, _, _) + } + + string toString() { none() } /** * Holds if this variable's value cannot be changed within a function. Currently used for string @@ -41,19 +47,19 @@ abstract class IRVariable extends TIRVariable { /** * Gets the type of the variable. */ - abstract Language::LanguageType getLanguageType(); + Language::LanguageType getLanguageType() { none() } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { none() } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { none() } /** * Gets the source location of this variable. @@ -72,7 +78,7 @@ abstract class IRVariable extends TIRVariable { } /** - * Represents a user-declared variable referenced by the IR for a function. + * A user-declared variable referenced by the IR for a function. */ class IRUserVariable extends IRVariable, TIRUserVariable { Language::Variable var; @@ -97,20 +103,34 @@ class IRUserVariable extends IRVariable, TIRUserVariable { } /** - * Represents a variable (user-declared or temporary) that is allocated on the - * stack. This includes all parameters, non-static local variables, and - * temporary variables. + * A variable (user-declared or temporary) that is allocated on the stack. This includes all + * parameters, non-static local variables, and temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { } +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + exists(Language::Variable var | + this = TIRUserVariable(var, _, func) and + Language::isVariableAutomatic(var) + ) + or + this = TIRTempVariable(func, _, _, _) + } +} +/** + * A user-declared variable that is allocated on the stack. This includes all parameters and + * non-static local variables. + */ class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { override Language::AutomaticVariable var; - IRAutomaticUserVariable() { Language::isVariableAutomatic(var) } - final override Language::AutomaticVariable getVariable() { result = var } } +/** + * A user-declared variable that is not allocated on the stack. This includes all global variables, + * namespace-scope variables, static fields, and static local variables. + */ class IRStaticUserVariable extends IRUserVariable { override Language::StaticVariable var; @@ -119,10 +139,19 @@ class IRStaticUserVariable extends IRUserVariable { final override Language::StaticVariable getVariable() { result = var } } -abstract class IRGeneratedVariable extends IRVariable { +/** + * A variable that is not user-declared. This includes temporary variables generated as part of IR + * construction, as well as string literals. + */ +class IRGeneratedVariable extends IRVariable { Language::AST ast; Language::LanguageType type; + IRGeneratedVariable() { + this = TIRTempVariable(func, ast, _, type) or + this = TIRStringLiteral(func, ast, type, _) + } + final override Language::LanguageType getLanguageType() { result = type } final override Language::AST getAST() { result = ast } @@ -144,6 +173,11 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } +/** + * A temporary variable introduced by IR construction. The most common examples are the variable + * generated to hold the return value of afunction, or the variable generated to hold the result of + * a condition operator (`a ? b : c`). + */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { TempVariableTag tag; @@ -158,18 +192,28 @@ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVa override string getBaseString() { result = "#temp" } } +/** + * A temporary variable generated to hold the return value of a function. + */ class IRReturnVariable extends IRTempVariable { IRReturnVariable() { tag = ReturnValueTempVar() } final override string toString() { result = "#return" } } +/** + * A temporary variable generated to hold the exception thrown by a `ThrowValue` instruction. + */ class IRThrowVariable extends IRTempVariable { IRThrowVariable() { tag = ThrowTempVar() } override string getBaseString() { result = "#throw" } } +/** + * A variable generated to represent the contents of a string literal. This variable acts much like + * a read-only global variable. + */ class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral { Language::StringLiteral literal; diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index e3c78f476e9..8819826d5c3 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -94,12 +94,21 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand( + Instruction instr, string message, IRFunction func, string funcText + ) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll index 6ecfd1470bb..bc67acc1be2 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll @@ -6,26 +6,6 @@ private import semmle.code.cpp.models.interfaces.Alias private class IntValue = Ints::IntValue; -/** - * Converts the bit count in `bits` to a byte count and a bit count in the form - * bytes:bits. - */ -bindingset[bits] -string bitsToBytesAndBits(int bits) { result = (bits / 8).toString() + ":" + (bits % 8).toString() } - -/** - * Gets a printable string for a bit offset with possibly unknown value. - */ -bindingset[bitOffset] -string getBitOffsetString(IntValue bitOffset) { - if Ints::hasValue(bitOffset) - then - if bitOffset >= 0 - then result = "+" + bitsToBytesAndBits(bitOffset) - else result = "-" + bitsToBytesAndBits(Ints::neg(bitOffset)) - else result = "+?" -} - /** * Gets the offset of field `field` in bits. */ @@ -137,7 +117,11 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) { or // Adding an integer to or subtracting an integer from a pointer propagates // the address with an offset. - bitOffset = getPointerBitOffset(instr.(PointerOffsetInstruction)) + exists(PointerOffsetInstruction ptrOffset | + ptrOffset = instr and + operand = ptrOffset.getLeftOperand() and + bitOffset = getPointerBitOffset(ptrOffset) + ) or // Computing a field address from a pointer propagates the address plus the // offset of the field. diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll index a789edc7590..96b4c28db82 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll @@ -865,3 +865,17 @@ private module CachedForDebugging { result.getTag() = var.getTag() } } + +module SSASanity { + query predicate multipleOperandMemoryLocations( + OldIR::MemoryOperand operand, string message, OldIR::IRFunction func, string funcText + ) { + exists(int locationCount | + locationCount = strictcount(Alias::getOperandMemoryLocation(operand)) and + locationCount > 1 and + func = operand.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) and + message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'." + ) + } +} diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql new file mode 100644 index 00000000000..3bd709a93dc --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql @@ -0,0 +1,8 @@ +/** + * @name Aliased SSA Sanity Check + * @description Performs sanity checks on the SSA construction. This query should have no results. + * @kind table + * @id cpp/aliased-ssa-sanity-check + */ + +import SSASanity diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll new file mode 100644 index 00000000000..95e8443b2a3 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll @@ -0,0 +1,2 @@ +private import SSAConstruction as SSA +import SSA::SSASanity diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll index e10e266b2ab..f9efad42fb0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll @@ -13,14 +13,20 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var } /** - * Represents a variable referenced by the IR for a function. The variable may - * be a user-declared variable (`IRUserVariable`) or a temporary variable - * generated by the AST-to-IR translation (`IRTempVariable`). + * A variable referenced by the IR for a function. The variable may be a user-declared variable + * (`IRUserVariable`) or a temporary variable generated by the AST-to-IR translation + * (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + IRVariable() { + this = TIRUserVariable(_, _, func) or + this = TIRTempVariable(func, _, _, _) or + this = TIRStringLiteral(func, _, _, _) + } + + string toString() { none() } /** * Holds if this variable's value cannot be changed within a function. Currently used for string @@ -41,19 +47,19 @@ abstract class IRVariable extends TIRVariable { /** * Gets the type of the variable. */ - abstract Language::LanguageType getLanguageType(); + Language::LanguageType getLanguageType() { none() } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { none() } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { none() } /** * Gets the source location of this variable. @@ -72,7 +78,7 @@ abstract class IRVariable extends TIRVariable { } /** - * Represents a user-declared variable referenced by the IR for a function. + * A user-declared variable referenced by the IR for a function. */ class IRUserVariable extends IRVariable, TIRUserVariable { Language::Variable var; @@ -97,20 +103,34 @@ class IRUserVariable extends IRVariable, TIRUserVariable { } /** - * Represents a variable (user-declared or temporary) that is allocated on the - * stack. This includes all parameters, non-static local variables, and - * temporary variables. + * A variable (user-declared or temporary) that is allocated on the stack. This includes all + * parameters, non-static local variables, and temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { } +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + exists(Language::Variable var | + this = TIRUserVariable(var, _, func) and + Language::isVariableAutomatic(var) + ) + or + this = TIRTempVariable(func, _, _, _) + } +} +/** + * A user-declared variable that is allocated on the stack. This includes all parameters and + * non-static local variables. + */ class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { override Language::AutomaticVariable var; - IRAutomaticUserVariable() { Language::isVariableAutomatic(var) } - final override Language::AutomaticVariable getVariable() { result = var } } +/** + * A user-declared variable that is not allocated on the stack. This includes all global variables, + * namespace-scope variables, static fields, and static local variables. + */ class IRStaticUserVariable extends IRUserVariable { override Language::StaticVariable var; @@ -119,10 +139,19 @@ class IRStaticUserVariable extends IRUserVariable { final override Language::StaticVariable getVariable() { result = var } } -abstract class IRGeneratedVariable extends IRVariable { +/** + * A variable that is not user-declared. This includes temporary variables generated as part of IR + * construction, as well as string literals. + */ +class IRGeneratedVariable extends IRVariable { Language::AST ast; Language::LanguageType type; + IRGeneratedVariable() { + this = TIRTempVariable(func, ast, _, type) or + this = TIRStringLiteral(func, ast, type, _) + } + final override Language::LanguageType getLanguageType() { result = type } final override Language::AST getAST() { result = ast } @@ -144,6 +173,11 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } +/** + * A temporary variable introduced by IR construction. The most common examples are the variable + * generated to hold the return value of afunction, or the variable generated to hold the result of + * a condition operator (`a ? b : c`). + */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { TempVariableTag tag; @@ -158,18 +192,28 @@ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVa override string getBaseString() { result = "#temp" } } +/** + * A temporary variable generated to hold the return value of a function. + */ class IRReturnVariable extends IRTempVariable { IRReturnVariable() { tag = ReturnValueTempVar() } final override string toString() { result = "#return" } } +/** + * A temporary variable generated to hold the exception thrown by a `ThrowValue` instruction. + */ class IRThrowVariable extends IRTempVariable { IRThrowVariable() { tag = ThrowTempVar() } override string getBaseString() { result = "#throw" } } +/** + * A variable generated to represent the contents of a string literal. This variable acts much like + * a read-only global variable. + */ class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral { Language::StringLiteral literal; diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index e3c78f476e9..8819826d5c3 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -94,12 +94,21 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand( + Instruction instr, string message, IRFunction func, string funcText + ) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll index e10e266b2ab..f9efad42fb0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll @@ -13,14 +13,20 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var } /** - * Represents a variable referenced by the IR for a function. The variable may - * be a user-declared variable (`IRUserVariable`) or a temporary variable - * generated by the AST-to-IR translation (`IRTempVariable`). + * A variable referenced by the IR for a function. The variable may be a user-declared variable + * (`IRUserVariable`) or a temporary variable generated by the AST-to-IR translation + * (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + IRVariable() { + this = TIRUserVariable(_, _, func) or + this = TIRTempVariable(func, _, _, _) or + this = TIRStringLiteral(func, _, _, _) + } + + string toString() { none() } /** * Holds if this variable's value cannot be changed within a function. Currently used for string @@ -41,19 +47,19 @@ abstract class IRVariable extends TIRVariable { /** * Gets the type of the variable. */ - abstract Language::LanguageType getLanguageType(); + Language::LanguageType getLanguageType() { none() } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { none() } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { none() } /** * Gets the source location of this variable. @@ -72,7 +78,7 @@ abstract class IRVariable extends TIRVariable { } /** - * Represents a user-declared variable referenced by the IR for a function. + * A user-declared variable referenced by the IR for a function. */ class IRUserVariable extends IRVariable, TIRUserVariable { Language::Variable var; @@ -97,20 +103,34 @@ class IRUserVariable extends IRVariable, TIRUserVariable { } /** - * Represents a variable (user-declared or temporary) that is allocated on the - * stack. This includes all parameters, non-static local variables, and - * temporary variables. + * A variable (user-declared or temporary) that is allocated on the stack. This includes all + * parameters, non-static local variables, and temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { } +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + exists(Language::Variable var | + this = TIRUserVariable(var, _, func) and + Language::isVariableAutomatic(var) + ) + or + this = TIRTempVariable(func, _, _, _) + } +} +/** + * A user-declared variable that is allocated on the stack. This includes all parameters and + * non-static local variables. + */ class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { override Language::AutomaticVariable var; - IRAutomaticUserVariable() { Language::isVariableAutomatic(var) } - final override Language::AutomaticVariable getVariable() { result = var } } +/** + * A user-declared variable that is not allocated on the stack. This includes all global variables, + * namespace-scope variables, static fields, and static local variables. + */ class IRStaticUserVariable extends IRUserVariable { override Language::StaticVariable var; @@ -119,10 +139,19 @@ class IRStaticUserVariable extends IRUserVariable { final override Language::StaticVariable getVariable() { result = var } } -abstract class IRGeneratedVariable extends IRVariable { +/** + * A variable that is not user-declared. This includes temporary variables generated as part of IR + * construction, as well as string literals. + */ +class IRGeneratedVariable extends IRVariable { Language::AST ast; Language::LanguageType type; + IRGeneratedVariable() { + this = TIRTempVariable(func, ast, _, type) or + this = TIRStringLiteral(func, ast, type, _) + } + final override Language::LanguageType getLanguageType() { result = type } final override Language::AST getAST() { result = ast } @@ -144,6 +173,11 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } +/** + * A temporary variable introduced by IR construction. The most common examples are the variable + * generated to hold the return value of afunction, or the variable generated to hold the result of + * a condition operator (`a ? b : c`). + */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { TempVariableTag tag; @@ -158,18 +192,28 @@ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVa override string getBaseString() { result = "#temp" } } +/** + * A temporary variable generated to hold the return value of a function. + */ class IRReturnVariable extends IRTempVariable { IRReturnVariable() { tag = ReturnValueTempVar() } final override string toString() { result = "#return" } } +/** + * A temporary variable generated to hold the exception thrown by a `ThrowValue` instruction. + */ class IRThrowVariable extends IRTempVariable { IRThrowVariable() { tag = ThrowTempVar() } override string getBaseString() { result = "#throw" } } +/** + * A variable generated to represent the contents of a string literal. This variable acts much like + * a read-only global variable. + */ class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral { Language::StringLiteral literal; diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index e3c78f476e9..8819826d5c3 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -94,12 +94,21 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand( + Instruction instr, string message, IRFunction func, string funcText + ) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll index 6ecfd1470bb..bc67acc1be2 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll @@ -6,26 +6,6 @@ private import semmle.code.cpp.models.interfaces.Alias private class IntValue = Ints::IntValue; -/** - * Converts the bit count in `bits` to a byte count and a bit count in the form - * bytes:bits. - */ -bindingset[bits] -string bitsToBytesAndBits(int bits) { result = (bits / 8).toString() + ":" + (bits % 8).toString() } - -/** - * Gets a printable string for a bit offset with possibly unknown value. - */ -bindingset[bitOffset] -string getBitOffsetString(IntValue bitOffset) { - if Ints::hasValue(bitOffset) - then - if bitOffset >= 0 - then result = "+" + bitsToBytesAndBits(bitOffset) - else result = "-" + bitsToBytesAndBits(Ints::neg(bitOffset)) - else result = "+?" -} - /** * Gets the offset of field `field` in bits. */ @@ -137,7 +117,11 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) { or // Adding an integer to or subtracting an integer from a pointer propagates // the address with an offset. - bitOffset = getPointerBitOffset(instr.(PointerOffsetInstruction)) + exists(PointerOffsetInstruction ptrOffset | + ptrOffset = instr and + operand = ptrOffset.getLeftOperand() and + bitOffset = getPointerBitOffset(ptrOffset) + ) or // Computing a field address from a pointer propagates the address plus the // offset of the field. diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll index a789edc7590..96b4c28db82 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll @@ -865,3 +865,17 @@ private module CachedForDebugging { result.getTag() = var.getTag() } } + +module SSASanity { + query predicate multipleOperandMemoryLocations( + OldIR::MemoryOperand operand, string message, OldIR::IRFunction func, string funcText + ) { + exists(int locationCount | + locationCount = strictcount(Alias::getOperandMemoryLocation(operand)) and + locationCount > 1 and + func = operand.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) and + message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'." + ) + } +} diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql new file mode 100644 index 00000000000..1b5ee80b603 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql @@ -0,0 +1,8 @@ +/** + * @name Unaliased SSA Sanity Check + * @description Performs sanity checks on the SSA construction. This query should have no results. + * @kind table + * @id cpp/unaliased-ssa-sanity-check + */ + +import SSASanity diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll new file mode 100644 index 00000000000..95e8443b2a3 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll @@ -0,0 +1,2 @@ +private import SSAConstruction as SSA +import SSA::SSASanity diff --git a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll index 55546ba380f..6034ebc5674 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll @@ -192,3 +192,33 @@ predicate isGT(IntValue a, IntValue b) { hasValue(a) and hasValue(b) and a > b } */ bindingset[a, b] predicate isGE(IntValue a, IntValue b) { hasValue(a) and hasValue(b) and a >= b } + +/** + * Converts the bit count in `bits` to a byte count and a bit count in the form + * "bytes:bits". If `bits` represents an integer number of bytes, the ":bits" section is omitted. + * If `bits` does not have a known value, the result is "?". + */ +bindingset[bits] +string bitsToBytesAndBits(IntValue bits) { + exists(int bytes, int leftoverBits | + hasValue(bits) and + bytes = bits / 8 and + leftoverBits = bits % 8 and + if leftoverBits = 0 then result = bytes.toString() else result = bytes + ":" + leftoverBits + ) + or + not hasValue(bits) and result = "?" +} + +/** + * Gets a printable string for a bit offset with possibly unknown value. + */ +bindingset[bitOffset] +string getBitOffsetString(IntValue bitOffset) { + if hasValue(bitOffset) + then + if bitOffset >= 0 + then result = "+" + bitsToBytesAndBits(bitOffset) + else result = "-" + bitsToBytesAndBits(neg(bitOffset)) + else result = "+?" +} diff --git a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll index bc09f9c2243..cd12b9b627a 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll @@ -30,5 +30,5 @@ Overlap getOverlap(IntValue defStart, IntValue defEnd, IntValue useStart, IntVal bindingset[start, end] string getIntervalString(IntValue start, IntValue end) { // We represent an interval has half-open, so print it as "[start..end)". - result = "[" + intValueToString(start) + ".." + intValueToString(end) + ")" + result = "[" + bitsToBytesAndBits(start) + ".." + bitsToBytesAndBits(end) + ")" } diff --git a/cpp/ql/test/library-tests/ir/escape/points_to.expected b/cpp/ql/test/library-tests/ir/escape/points_to.expected index ac518b5dd79..6447bd81a5f 100644 --- a/cpp/ql/test/library-tests/ir/escape/points_to.expected +++ b/cpp/ql/test/library-tests/ir/escape/points_to.expected @@ -1,86 +1,86 @@ -| escape.cpp:111:18:111:21 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:115:19:115:28 | PointerAdd[4] | no_+0:0 | no_+0:0 | -| escape.cpp:115:20:115:23 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:116:19:116:28 | PointerSub[4] | no_+0:0 | no_+0:0 | -| escape.cpp:116:20:116:23 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:117:19:117:26 | PointerAdd[4] | no_+0:0 | no_+0:0 | -| escape.cpp:117:23:117:26 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:118:9:118:12 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:120:12:120:15 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:123:14:123:17 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:124:15:124:18 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:127:9:127:12 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:129:12:129:15 | CopyValue | no_+0:0 | no_+0:0 | -| escape.cpp:134:5:134:18 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:134:11:134:18 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:135:5:135:12 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:135:5:135:15 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:136:5:136:15 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:136:7:136:14 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:137:17:137:24 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:137:17:137:27 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:138:17:138:27 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:138:19:138:26 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:140:21:140:32 | FieldAddress[x] | no_Point+0:0 | no_Point+0:0 | -| escape.cpp:140:21:140:32 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:140:21:140:32 | FieldAddress[z] | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:141:27:141:27 | FieldAddress[x] | no_Point+0:0 | no_Point+0:0 | -| escape.cpp:142:14:142:14 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:143:19:143:27 | CopyValue | no_Point+0:0 | no_Point+0:0 | -| escape.cpp:143:31:143:31 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:144:6:144:14 | CopyValue | no_Point+0:0 | no_Point+0:0 | -| escape.cpp:144:18:144:18 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:145:20:145:30 | CopyValue | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:145:30:145:30 | FieldAddress[z] | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:146:5:146:18 | CopyValue | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:146:7:146:17 | CopyValue | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:146:17:146:17 | FieldAddress[z] | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:149:5:149:14 | ConvertToNonVirtualBase[Derived : Intermediate1] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:149:5:149:14 | ConvertToNonVirtualBase[Intermediate1 : Base] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:149:16:149:16 | FieldAddress[b] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:150:18:150:27 | ConvertToNonVirtualBase[Derived : Intermediate1] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:150:18:150:27 | ConvertToNonVirtualBase[Intermediate1 : Base] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:150:29:150:29 | FieldAddress[b] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:151:5:151:14 | ConvertToNonVirtualBase[Derived : Intermediate2] | no_Derived+12:0 | no_Derived+12:0 | -| escape.cpp:151:16:151:17 | FieldAddress[i2] | no_Derived+16:0 | no_Derived+16:0 | -| escape.cpp:152:19:152:28 | ConvertToNonVirtualBase[Derived : Intermediate2] | no_Derived+12:0 | no_Derived+12:0 | -| escape.cpp:152:30:152:31 | FieldAddress[i2] | no_Derived+16:0 | no_Derived+16:0 | -| escape.cpp:155:17:155:30 | CopyValue | no_ssa_addrOf+0:0 | no_ssa_addrOf+0:0 | -| escape.cpp:155:17:155:30 | Store | no_ssa_addrOf+0:0 | no_ssa_addrOf+0:0 | -| escape.cpp:158:17:158:28 | CopyValue | no_ssa_refTo+0:0 | no_ssa_refTo+0:0 | -| escape.cpp:158:17:158:28 | Store | no_ssa_refTo+0:0 | no_ssa_refTo+0:0 | -| escape.cpp:161:19:161:42 | Convert | no_ssa_refToArrayElement+0:0 | no_ssa_refToArrayElement+0:0 | -| escape.cpp:161:19:161:45 | CopyValue | no_ssa_refToArrayElement+20:0 | no_ssa_refToArrayElement+20:0 | -| escape.cpp:161:19:161:45 | PointerAdd[4] | no_ssa_refToArrayElement+20:0 | no_ssa_refToArrayElement+20:0 | -| escape.cpp:161:19:161:45 | Store | no_ssa_refToArrayElement+20:0 | no_ssa_refToArrayElement+20:0 | -| escape.cpp:164:24:164:40 | CopyValue | no_ssa_refToArray+0:0 | no_ssa_refToArray+0:0 | -| escape.cpp:164:24:164:40 | Store | no_ssa_refToArray+0:0 | no_ssa_refToArray+0:0 | -| escape.cpp:167:19:167:28 | CopyValue | passByPtr+0:0 | passByPtr+0:0 | -| escape.cpp:170:21:170:29 | CopyValue | passByRef+0:0 | passByRef+0:0 | -| escape.cpp:173:22:173:38 | CopyValue | no_ssa_passByPtr+0:0 | no_ssa_passByPtr+0:0 | -| escape.cpp:176:24:176:39 | CopyValue | no_ssa_passByRef+0:0 | no_ssa_passByRef+0:0 | -| escape.cpp:179:22:179:42 | CopyValue | no_ssa_passByPtr_ret+0:0 | no_ssa_passByPtr_ret+0:0 | -| escape.cpp:182:24:182:43 | CopyValue | no_ssa_passByRef_ret+0:0 | no_ssa_passByRef_ret+0:0 | -| escape.cpp:185:30:185:40 | CopyValue | passByPtr2+0:0 | passByPtr2+0:0 | -| escape.cpp:188:32:188:41 | CopyValue | passByRef2+0:0 | passByRef2+0:0 | -| escape.cpp:191:30:191:42 | Call | none | passByPtr3+0:0 | -| escape.cpp:191:44:191:54 | CopyValue | passByPtr3+0:0 | passByPtr3+0:0 | -| escape.cpp:194:32:194:46 | Call | none | passByRef3+0:0 | -| escape.cpp:194:32:194:59 | CopyValue | none | passByRef3+0:0 | -| escape.cpp:194:48:194:57 | CopyValue | passByRef3+0:0 | passByRef3+0:0 | -| escape.cpp:199:17:199:34 | CopyValue | no_ssa_passByPtr4+0:0 | no_ssa_passByPtr4+0:0 | -| escape.cpp:199:37:199:54 | CopyValue | no_ssa_passByPtr5+0:0 | no_ssa_passByPtr5+0:0 | -| escape.cpp:202:5:202:19 | Call | none | passByRef6+0:0 | -| escape.cpp:202:5:202:32 | CopyValue | none | passByRef6+0:0 | -| escape.cpp:202:21:202:30 | CopyValue | passByRef6+0:0 | passByRef6+0:0 | -| escape.cpp:205:5:205:19 | Call | none | no_ssa_passByRef7+0:0 | -| escape.cpp:205:5:205:39 | CopyValue | none | no_ssa_passByRef7+0:0 | -| escape.cpp:205:21:205:37 | CopyValue | no_ssa_passByRef7+0:0 | no_ssa_passByRef7+0:0 | -| escape.cpp:209:14:209:25 | Call | none | no_ssa_c+0:0 | -| escape.cpp:217:14:217:16 | CopyValue | c2+0:0 | c2+0:0 | -| escape.cpp:221:8:221:19 | Call | none | c3+0:0 | -| escape.cpp:225:17:225:28 | Call | none | c4+0:0 | -| escape.cpp:247:2:247:27 | Store | condEscape1+0:0 | condEscape1+0:0 | -| escape.cpp:247:16:247:27 | CopyValue | condEscape1+0:0 | condEscape1+0:0 | -| escape.cpp:249:9:249:34 | Store | condEscape2+0:0 | condEscape2+0:0 | -| escape.cpp:249:23:249:34 | CopyValue | condEscape2+0:0 | condEscape2+0:0 | +| escape.cpp:111:18:111:21 | CopyValue | no_+0 | no_+0 | +| escape.cpp:115:19:115:28 | PointerAdd[4] | no_+0 | no_+0 | +| escape.cpp:115:20:115:23 | CopyValue | no_+0 | no_+0 | +| escape.cpp:116:19:116:28 | PointerSub[4] | no_+0 | no_+0 | +| escape.cpp:116:20:116:23 | CopyValue | no_+0 | no_+0 | +| escape.cpp:117:19:117:26 | PointerAdd[4] | no_+0 | no_+0 | +| escape.cpp:117:23:117:26 | CopyValue | no_+0 | no_+0 | +| escape.cpp:118:9:118:12 | CopyValue | no_+0 | no_+0 | +| escape.cpp:120:12:120:15 | CopyValue | no_+0 | no_+0 | +| escape.cpp:123:14:123:17 | CopyValue | no_+0 | no_+0 | +| escape.cpp:124:15:124:18 | CopyValue | no_+0 | no_+0 | +| escape.cpp:127:9:127:12 | CopyValue | no_+0 | no_+0 | +| escape.cpp:129:12:129:15 | CopyValue | no_+0 | no_+0 | +| escape.cpp:134:5:134:18 | Convert | no_Array+0 | no_Array+0 | +| escape.cpp:134:11:134:18 | Convert | no_Array+0 | no_Array+0 | +| escape.cpp:135:5:135:12 | Convert | no_Array+0 | no_Array+0 | +| escape.cpp:135:5:135:15 | PointerAdd[4] | no_Array+20 | no_Array+20 | +| escape.cpp:136:5:136:15 | PointerAdd[4] | no_Array+20 | no_Array+20 | +| escape.cpp:136:7:136:14 | Convert | no_Array+0 | no_Array+0 | +| escape.cpp:137:17:137:24 | Convert | no_Array+0 | no_Array+0 | +| escape.cpp:137:17:137:27 | PointerAdd[4] | no_Array+20 | no_Array+20 | +| escape.cpp:138:17:138:27 | PointerAdd[4] | no_Array+20 | no_Array+20 | +| escape.cpp:138:19:138:26 | Convert | no_Array+0 | no_Array+0 | +| escape.cpp:140:21:140:32 | FieldAddress[x] | no_Point+0 | no_Point+0 | +| escape.cpp:140:21:140:32 | FieldAddress[y] | no_Point+4 | no_Point+4 | +| escape.cpp:140:21:140:32 | FieldAddress[z] | no_Point+8 | no_Point+8 | +| escape.cpp:141:27:141:27 | FieldAddress[x] | no_Point+0 | no_Point+0 | +| escape.cpp:142:14:142:14 | FieldAddress[y] | no_Point+4 | no_Point+4 | +| escape.cpp:143:19:143:27 | CopyValue | no_Point+0 | no_Point+0 | +| escape.cpp:143:31:143:31 | FieldAddress[y] | no_Point+4 | no_Point+4 | +| escape.cpp:144:6:144:14 | CopyValue | no_Point+0 | no_Point+0 | +| escape.cpp:144:18:144:18 | FieldAddress[y] | no_Point+4 | no_Point+4 | +| escape.cpp:145:20:145:30 | CopyValue | no_Point+8 | no_Point+8 | +| escape.cpp:145:30:145:30 | FieldAddress[z] | no_Point+8 | no_Point+8 | +| escape.cpp:146:5:146:18 | CopyValue | no_Point+8 | no_Point+8 | +| escape.cpp:146:7:146:17 | CopyValue | no_Point+8 | no_Point+8 | +| escape.cpp:146:17:146:17 | FieldAddress[z] | no_Point+8 | no_Point+8 | +| escape.cpp:149:5:149:14 | ConvertToNonVirtualBase[Derived : Intermediate1] | no_Derived+0 | no_Derived+0 | +| escape.cpp:149:5:149:14 | ConvertToNonVirtualBase[Intermediate1 : Base] | no_Derived+0 | no_Derived+0 | +| escape.cpp:149:16:149:16 | FieldAddress[b] | no_Derived+0 | no_Derived+0 | +| escape.cpp:150:18:150:27 | ConvertToNonVirtualBase[Derived : Intermediate1] | no_Derived+0 | no_Derived+0 | +| escape.cpp:150:18:150:27 | ConvertToNonVirtualBase[Intermediate1 : Base] | no_Derived+0 | no_Derived+0 | +| escape.cpp:150:29:150:29 | FieldAddress[b] | no_Derived+0 | no_Derived+0 | +| escape.cpp:151:5:151:14 | ConvertToNonVirtualBase[Derived : Intermediate2] | no_Derived+12 | no_Derived+12 | +| escape.cpp:151:16:151:17 | FieldAddress[i2] | no_Derived+16 | no_Derived+16 | +| escape.cpp:152:19:152:28 | ConvertToNonVirtualBase[Derived : Intermediate2] | no_Derived+12 | no_Derived+12 | +| escape.cpp:152:30:152:31 | FieldAddress[i2] | no_Derived+16 | no_Derived+16 | +| escape.cpp:155:17:155:30 | CopyValue | no_ssa_addrOf+0 | no_ssa_addrOf+0 | +| escape.cpp:155:17:155:30 | Store | no_ssa_addrOf+0 | no_ssa_addrOf+0 | +| escape.cpp:158:17:158:28 | CopyValue | no_ssa_refTo+0 | no_ssa_refTo+0 | +| escape.cpp:158:17:158:28 | Store | no_ssa_refTo+0 | no_ssa_refTo+0 | +| escape.cpp:161:19:161:42 | Convert | no_ssa_refToArrayElement+0 | no_ssa_refToArrayElement+0 | +| escape.cpp:161:19:161:45 | CopyValue | no_ssa_refToArrayElement+20 | no_ssa_refToArrayElement+20 | +| escape.cpp:161:19:161:45 | PointerAdd[4] | no_ssa_refToArrayElement+20 | no_ssa_refToArrayElement+20 | +| escape.cpp:161:19:161:45 | Store | no_ssa_refToArrayElement+20 | no_ssa_refToArrayElement+20 | +| escape.cpp:164:24:164:40 | CopyValue | no_ssa_refToArray+0 | no_ssa_refToArray+0 | +| escape.cpp:164:24:164:40 | Store | no_ssa_refToArray+0 | no_ssa_refToArray+0 | +| escape.cpp:167:19:167:28 | CopyValue | passByPtr+0 | passByPtr+0 | +| escape.cpp:170:21:170:29 | CopyValue | passByRef+0 | passByRef+0 | +| escape.cpp:173:22:173:38 | CopyValue | no_ssa_passByPtr+0 | no_ssa_passByPtr+0 | +| escape.cpp:176:24:176:39 | CopyValue | no_ssa_passByRef+0 | no_ssa_passByRef+0 | +| escape.cpp:179:22:179:42 | CopyValue | no_ssa_passByPtr_ret+0 | no_ssa_passByPtr_ret+0 | +| escape.cpp:182:24:182:43 | CopyValue | no_ssa_passByRef_ret+0 | no_ssa_passByRef_ret+0 | +| escape.cpp:185:30:185:40 | CopyValue | passByPtr2+0 | passByPtr2+0 | +| escape.cpp:188:32:188:41 | CopyValue | passByRef2+0 | passByRef2+0 | +| escape.cpp:191:30:191:42 | Call | none | passByPtr3+0 | +| escape.cpp:191:44:191:54 | CopyValue | passByPtr3+0 | passByPtr3+0 | +| escape.cpp:194:32:194:46 | Call | none | passByRef3+0 | +| escape.cpp:194:32:194:59 | CopyValue | none | passByRef3+0 | +| escape.cpp:194:48:194:57 | CopyValue | passByRef3+0 | passByRef3+0 | +| escape.cpp:199:17:199:34 | CopyValue | no_ssa_passByPtr4+0 | no_ssa_passByPtr4+0 | +| escape.cpp:199:37:199:54 | CopyValue | no_ssa_passByPtr5+0 | no_ssa_passByPtr5+0 | +| escape.cpp:202:5:202:19 | Call | none | passByRef6+0 | +| escape.cpp:202:5:202:32 | CopyValue | none | passByRef6+0 | +| escape.cpp:202:21:202:30 | CopyValue | passByRef6+0 | passByRef6+0 | +| escape.cpp:205:5:205:19 | Call | none | no_ssa_passByRef7+0 | +| escape.cpp:205:5:205:39 | CopyValue | none | no_ssa_passByRef7+0 | +| escape.cpp:205:21:205:37 | CopyValue | no_ssa_passByRef7+0 | no_ssa_passByRef7+0 | +| escape.cpp:209:14:209:25 | Call | none | no_ssa_c+0 | +| escape.cpp:217:14:217:16 | CopyValue | c2+0 | c2+0 | +| escape.cpp:221:8:221:19 | Call | none | c3+0 | +| escape.cpp:225:17:225:28 | Call | none | c4+0 | +| escape.cpp:247:2:247:27 | Store | condEscape1+0 | condEscape1+0 | +| escape.cpp:247:16:247:27 | CopyValue | condEscape1+0 | condEscape1+0 | +| escape.cpp:249:9:249:34 | Store | condEscape2+0 | condEscape2+0 | +| escape.cpp:249:23:249:34 | CopyValue | condEscape2+0 | condEscape2+0 | diff --git a/cpp/ql/test/library-tests/ir/escape/points_to.ql b/cpp/ql/test/library-tests/ir/escape/points_to.ql index 04dbf0e0060..7c265974b10 100644 --- a/cpp/ql/test/library-tests/ir/escape/points_to.ql +++ b/cpp/ql/test/library-tests/ir/escape/points_to.ql @@ -4,6 +4,7 @@ import semmle.code.cpp.ir.implementation.raw.IR as Raw import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasAnalysis as UnAA import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as Un import semmle.code.cpp.ir.implementation.unaliased_ssa.internal.SSAConstruction +import semmle.code.cpp.ir.internal.IntegerConstant from Raw::Instruction rawInstr, Un::Instruction unInstr, string rawPointsTo, string unPointsTo where @@ -12,21 +13,21 @@ where ( exists(Variable var, int rawBitOffset, int unBitOffset | RawAA::resultPointsTo(rawInstr, Raw::getIRUserVariable(_, var), rawBitOffset) and - rawPointsTo = var.toString() + RawAA::getBitOffsetString(rawBitOffset) and + rawPointsTo = var.toString() + getBitOffsetString(rawBitOffset) and UnAA::resultPointsTo(unInstr, Un::getIRUserVariable(_, var), unBitOffset) and - unPointsTo = var.toString() + UnAA::getBitOffsetString(unBitOffset) + unPointsTo = var.toString() + getBitOffsetString(unBitOffset) ) or exists(Variable var, int unBitOffset | not RawAA::resultPointsTo(rawInstr, Raw::getIRUserVariable(_, var), _) and rawPointsTo = "none" and UnAA::resultPointsTo(unInstr, Un::getIRUserVariable(_, var), unBitOffset) and - unPointsTo = var.toString() + UnAA::getBitOffsetString(unBitOffset) + unPointsTo = var.toString() + getBitOffsetString(unBitOffset) ) or exists(Variable var, int rawBitOffset | RawAA::resultPointsTo(rawInstr, Raw::getIRUserVariable(_, var), rawBitOffset) and - rawPointsTo = var.toString() + RawAA::getBitOffsetString(rawBitOffset) and + rawPointsTo = var.toString() + getBitOffsetString(rawBitOffset) and not UnAA::resultPointsTo(unInstr, Un::getIRUserVariable(_, var), _) and unPointsTo = "none" ) diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref new file mode 100644 index 00000000000..8e348011785 --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref new file mode 100644 index 00000000000..18bf9212dbf --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref new file mode 100644 index 00000000000..8e348011785 --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref new file mode 100644 index 00000000000..18bf9212dbf --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected index a33f824484b..972b06af73c 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected @@ -1,4 +1,6 @@ -| test.c:15:20:15:25 | call to malloc | This allocation does not include space to null-terminate the string. | -| test.c:29:20:29:25 | call to malloc | This allocation does not include space to null-terminate the string. | -| test.c:44:20:44:25 | call to malloc | This allocation does not include space to null-terminate the string. | -| test.cpp:18:35:18:40 | call to malloc | This allocation does not include space to null-terminate the string. | +| test.c:16:20:16:25 | call to malloc | This allocation does not include space to null-terminate the string. | +| test.c:32:20:32:25 | call to malloc | This allocation does not include space to null-terminate the string. | +| test.c:49:20:49:25 | call to malloc | This allocation does not include space to null-terminate the string. | +| test.cpp:24:35:24:40 | call to malloc | This allocation does not include space to null-terminate the string. | +| test.cpp:63:28:63:33 | call to malloc | This allocation does not include space to null-terminate the string. | +| test.cpp:71:28:71:33 | call to malloc | This allocation does not include space to null-terminate the string. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c index a6d054874d0..2f06af7d42d 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c @@ -7,18 +7,21 @@ typedef unsigned long size_t; void *malloc(size_t size); void free(void *ptr); +char *strcpy(char *s1, const char *s2); //// Test code ///// void bad0(char *str) { // BAD -- Not allocating space for '\0' terminator char *buffer = malloc(strlen(str)); + strcpy(buffer, str); free(buffer); } void good0(char *str) { // GOOD -- Allocating extra byte for terminator char *buffer = malloc(strlen(str)+1); + strcpy(buffer, str); free(buffer); } @@ -27,6 +30,7 @@ void bad1(char *str) { int len = strlen(str); // BAD -- Not allocating space for '\0' terminator char *buffer = malloc(len); + strcpy(buffer, str); free(buffer); } @@ -34,6 +38,7 @@ void good1(char *str) { int len = strlen(str); // GOOD -- Allocating extra byte for terminator char *buffer = malloc(len+1); + strcpy(buffer, str); free(buffer); } @@ -42,6 +47,7 @@ void bad2(char *str) { int len = strlen(str); // BAD -- Not allocating space for '\0' terminator char *buffer = malloc(len); + strcpy(buffer, str); free(buffer); } @@ -49,18 +55,21 @@ void good2(char *str) { int len = strlen(str)+1; // GOOD -- Allocating extra byte for terminator char *buffer = malloc(len); + strcpy(buffer, str); free(buffer); } void bad3(char *str) { // BAD -- Not allocating space for '\0' terminator [NOT DETECTED] char *buffer = malloc(strlen(str) * sizeof(char)); + strcpy(buffer, str); free(buffer); } void good3(char *str) { // GOOD -- Allocating extra byte for terminator char *buffer = malloc((strlen(str) + 1) * sizeof(char)); + strcpy(buffer, str); free(buffer); } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.cpp index 53488c0f229..23c446b2a84 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.cpp @@ -10,23 +10,100 @@ typedef unsigned long size_t; void *malloc(size_t size); void free(void *ptr); size_t wcslen(const wchar_t *s); +wchar_t* wcscpy(wchar_t* s1, const wchar_t* s2); +int sprintf(char *s, const char *format, ...); +int wprintf(const wchar_t *format, ...); +char *strcat(char *s1, const char *s2); +size_t strlen(const char *s); +int strcmp(const char *s1, const char *s2); //// Test code ///// void bad1(wchar_t *wstr) { // BAD -- Not allocating space for '\0' terminator wchar_t *wbuffer = (wchar_t *)malloc(wcslen(wstr)); + wcscpy(wbuffer, wstr); free(wbuffer); } void bad2(wchar_t *wstr) { // BAD -- Not allocating space for '\0' terminator [NOT DETECTED] wchar_t *wbuffer = (wchar_t *)malloc(wcslen(wstr) * sizeof(wchar_t)); + wcscpy(wbuffer, wstr); free(wbuffer); } void good1(wchar_t *wstr) { // GOOD -- Allocating extra character for terminator wchar_t *wbuffer = (wchar_t *)malloc((wcslen(wstr) + 1) * sizeof(wchar_t)); + wcscpy(wbuffer, wstr); free(wbuffer); } + +void bad3(char *str) { + // BAD -- zero-termination proved by sprintf (as destination) [NOT DETECTED] + char *buffer = (char *)malloc(strlen(str)); + sprintf(buffer, "%s", str); + free(buffer); +} + +void decode(char *dest, char *src); +void wdecode(wchar_t *dest, wchar_t *src); + +void bad4(char *str) { + // BAD -- zero-termination proved by wprintf (as parameter) [NOT DETECTED] + char *buffer = (char *)malloc(strlen(str)); + decode(buffer, str); + wprintf(L"%s", buffer); + free(buffer); +} + +void bad5(char *str) { + // BAD -- zero-termination proved by strcat (as destination) + char *buffer = (char *)malloc(strlen(str)); + buffer[0] = 0; + strcat(buffer, str); + free(buffer); +} + +void bad6(char *str, char *dest) { + // BAD -- zero-termination proved by strcat (as source) + char *buffer = (char *)malloc(strlen(str)); + decode(buffer, str); + strcat(dest, buffer); + free(buffer); +} + +void bad7(char *str, char *str2) { + // BAD -- zero-termination proved by strcmp [NOT DETECTED] + char *buffer = (char *)malloc(strlen(str)); + decode(buffer, str); + if (strcmp(buffer, str2) == 0) { + // ... + } + free(buffer); +} + +void bad8(wchar_t *str) { + // BAD -- zero-termination proved by wcslen [NOT DETECTED] + wchar_t *wbuffer = (wchar_t *)malloc(wcslen(str)); + wdecode(wbuffer, str); + if (wcslen(wbuffer) == 0) { + // ... + } + free(wbuffer); +} + +void good2(char *str, char *dest) { + // GOOD -- zero-termination not proven + char *buffer = (char *)malloc(strlen(str)); + decode(buffer, str); + free(buffer); +} + +void bad9(wchar_t *wstr) { + // BAD -- using new [NOT DETECTED] + wchar_t *wbuffer = new wchar_t[wcslen(wstr)]; + wcscpy(wbuffer, wstr); + delete wbuffer; +} diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test2.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test2.cpp new file mode 100644 index 00000000000..ec71fa5fa57 --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test2.cpp @@ -0,0 +1,50 @@ + +///// Library functions ////// + +typedef unsigned long size_t; + +void *malloc(size_t size); +void free(void *ptr); +size_t strlen(const char *s); + +namespace std +{ + template struct char_traits; + + template class allocator { + public: + allocator() throw(); + typedef size_t size_type; + }; + + template, class Allocator = allocator > + class basic_string { + public: + typedef typename Allocator::size_type size_type; + explicit basic_string(const Allocator& a = Allocator()); + basic_string(const charT* s, const Allocator& a = Allocator()); + basic_string(const charT* s, size_type n, const Allocator& a = Allocator()); + + const charT* c_str() const; + }; + + typedef basic_string string; +} + +//// Test code ///// + +void bad1(char *str) { + // BAD -- Not allocating space for '\0' terminator [NOT DETECTED] + char *buffer = (char *)malloc(strlen(str)); + std::string str2(buffer); + free(buffer); +} + +void good1(char *str) { + // GOOD --- copy does not overrun due to size limit + char *buffer = (char *)malloc(strlen(str)); + std::string str2(buffer, strlen(str)); + free(buffer); +} + + diff --git a/csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs b/csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs index 20d19a3525c..a2e59cf7ac3 100644 --- a/csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs +++ b/csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs @@ -185,7 +185,7 @@ namespace Semmle.Extraction.Tests // Records the arguments passed to StartCallback. IList StartCallbackIn = new List(); - void StartCallback(string s) + void StartCallback(string s, bool silent) { StartCallbackIn.Add(s); } @@ -194,7 +194,7 @@ namespace Semmle.Extraction.Tests IList EndCallbackIn = new List(); IList EndCallbackReturn = new List(); - void EndCallback(int ret, string s) + void EndCallback(int ret, string s, bool silent) { EndCallbackReturn.Add(ret); EndCallbackIn.Add(s); @@ -203,7 +203,7 @@ namespace Semmle.Extraction.Tests [Fact] public void TestBuildCommand() { - var cmd = BuildScript.Create("abc", "def ghi", null, null); + var cmd = BuildScript.Create("abc", "def ghi", false, null, null); Actions.RunProcess["abc def ghi"] = 1; cmd.Run(Actions, StartCallback, EndCallback); @@ -216,7 +216,7 @@ namespace Semmle.Extraction.Tests [Fact] public void TestAnd1() { - var cmd = BuildScript.Create("abc", "def ghi", null, null) & BuildScript.Create("odasa", null, null, null); + var cmd = BuildScript.Create("abc", "def ghi", false, null, null) & BuildScript.Create("odasa", null, false, null, null); Actions.RunProcess["abc def ghi"] = 1; cmd.Run(Actions, StartCallback, EndCallback); @@ -230,7 +230,7 @@ namespace Semmle.Extraction.Tests [Fact] public void TestAnd2() { - var cmd = BuildScript.Create("odasa", null, null, null) & BuildScript.Create("abc", "def ghi", null, null); + var cmd = BuildScript.Create("odasa", null, false, null, null) & BuildScript.Create("abc", "def ghi", false, null, null); Actions.RunProcess["abc def ghi"] = 1; Actions.RunProcess["odasa "] = 0; @@ -250,7 +250,7 @@ namespace Semmle.Extraction.Tests [Fact] public void TestOr1() { - var cmd = BuildScript.Create("odasa", null, null, null) | BuildScript.Create("abc", "def ghi", null, null); + var cmd = BuildScript.Create("odasa", null, false, null, null) | BuildScript.Create("abc", "def ghi", false, null, null); Actions.RunProcess["abc def ghi"] = 1; Actions.RunProcess["odasa "] = 0; @@ -266,7 +266,7 @@ namespace Semmle.Extraction.Tests [Fact] public void TestOr2() { - var cmd = BuildScript.Create("abc", "def ghi", null, null) | BuildScript.Create("odasa", null, null, null); + var cmd = BuildScript.Create("abc", "def ghi", false, null, null) | BuildScript.Create("odasa", null, false, null, null); Actions.RunProcess["abc def ghi"] = 1; Actions.RunProcess["odasa "] = 0; @@ -375,7 +375,7 @@ namespace Semmle.Extraction.Tests Actions.RunProcess["cmd.exe /C dotnet --info"] = 0; Actions.RunProcess["cmd.exe /C dotnet clean test.csproj"] = 0; Actions.RunProcess["cmd.exe /C dotnet restore test.csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false test.csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto dotnet build --no-incremental test.csproj"] = 0; Actions.RunProcess[@"cmd.exe /C C:\codeql\tools\java\bin\java -jar C:\codeql\csharp\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; @@ -401,6 +401,9 @@ namespace Semmle.Extraction.Tests [Fact] public void TestLinuxCSharpAutoBuilder() { + Actions.RunProcess["dotnet --list-runtimes"] = 0; + Actions.RunProcessOut["dotnet --list-runtimes"] = @"Microsoft.AspNetCore.App 2.2.5 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] +Microsoft.NETCore.App 2.2.5 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]"; Actions.RunProcess["dotnet --info"] = 0; Actions.RunProcess["dotnet clean test.csproj"] = 0; Actions.RunProcess["dotnet restore test.csproj"] = 0; @@ -424,7 +427,7 @@ namespace Semmle.Extraction.Tests Actions.LoadXml["test.csproj"] = xml; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 7); } [Fact] @@ -598,6 +601,8 @@ namespace Semmle.Extraction.Tests [Fact] public void TestLinuxBuildCommand() { + Actions.RunProcess["dotnet --list-runtimes"] = 1; + Actions.RunProcessOut["dotnet --list-runtimes"] = ""; Actions.RunProcess[@"C:\odasa/tools/odasa index --auto ""./build.sh --skip-tests"""] = 0; Actions.RunProcess[@"C:\codeql\tools\java/bin/java -jar C:\codeql\csharp/tools/extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa/tools/odasa index --xml --extensions config csproj props xml"] = 0; @@ -610,7 +615,7 @@ namespace Semmle.Extraction.Tests SkipVsWhere(); var autobuilder = CreateAutoBuilder("csharp", false, buildCommand: "./build.sh --skip-tests"); - TestAutobuilderScript(autobuilder, 0, 3); + TestAutobuilderScript(autobuilder, 0, 4); } [Fact] @@ -621,6 +626,8 @@ namespace Semmle.Extraction.Tests Actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = ""; Actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = ""; Actions.RunProcess["/bin/chmod u+x build/build.sh"] = 0; + Actions.RunProcess["dotnet --list-runtimes"] = 1; + Actions.RunProcessOut["dotnet --list-runtimes"] = ""; Actions.RunProcess[@"C:\odasa/tools/odasa index --auto build/build.sh"] = 0; Actions.RunProcessWorkingDirectory[@"C:\odasa/tools/odasa index --auto build/build.sh"] = "build"; Actions.RunProcess[@"C:\codeql\tools\java/bin/java -jar C:\codeql\csharp/tools/extractor-asp.jar ."] = 0; @@ -628,7 +635,7 @@ namespace Semmle.Extraction.Tests Actions.FileExists["csharp.log"] = true; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 0, 4); + TestAutobuilderScript(autobuilder, 0, 5); } [Fact] @@ -640,12 +647,14 @@ namespace Semmle.Extraction.Tests Actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = ""; Actions.RunProcess["/bin/chmod u+x build.sh"] = 0; + Actions.RunProcess["dotnet --list-runtimes"] = 1; + Actions.RunProcessOut["dotnet --list-runtimes"] = ""; Actions.RunProcess[@"C:\odasa/tools/odasa index --auto build.sh"] = 0; Actions.RunProcessWorkingDirectory[@"C:\odasa/tools/odasa index --auto build.sh"] = ""; Actions.FileExists["csharp.log"] = false; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 1, 2); + TestAutobuilderScript(autobuilder, 1, 3); } [Fact] @@ -657,12 +666,14 @@ namespace Semmle.Extraction.Tests Actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = ""; Actions.RunProcess["/bin/chmod u+x build.sh"] = 0; + Actions.RunProcess["dotnet --list-runtimes"] = 1; + Actions.RunProcessOut["dotnet --list-runtimes"] = ""; Actions.RunProcess[@"C:\odasa/tools/odasa index --auto build.sh"] = 5; Actions.RunProcessWorkingDirectory[@"C:\odasa/tools/odasa index --auto build.sh"] = ""; Actions.FileExists["csharp.log"] = true; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 1, 2); + TestAutobuilderScript(autobuilder, 1, 3); } [Fact] @@ -748,7 +759,7 @@ namespace Semmle.Extraction.Tests TestAutobuilderScript(autobuilder, 0, 6); } - [Fact] + [Fact] public void TestWindowCSharpMsBuildMultipleSolutions() { Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\csharp\nuget\nuget.exe restore test1.csproj"] = 0; @@ -871,6 +882,9 @@ namespace Semmle.Extraction.Tests [Fact] public void TestSkipNugetDotnet() { + Actions.RunProcess["dotnet --list-runtimes"] = 0; + Actions.RunProcessOut["dotnet --list-runtimes"] = @"Microsoft.AspNetCore.App 2.1.3 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] +Microsoft.NETCore.App 2.1.3 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]"; Actions.RunProcess["dotnet --info"] = 0; Actions.RunProcess["dotnet clean test.csproj"] = 0; Actions.RunProcess["dotnet restore test.csproj"] = 0; @@ -894,7 +908,7 @@ namespace Semmle.Extraction.Tests Actions.LoadXml["test.csproj"] = xml; var autobuilder = CreateAutoBuilder("csharp", false, dotnetArguments: "--no-restore"); // nugetRestore=false does not work for now. - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 7); } [Fact] @@ -906,10 +920,13 @@ namespace Semmle.Extraction.Tests Actions.RunProcess[@"chmod u+x dotnet-install.sh"] = 0; Actions.RunProcess[@"./dotnet-install.sh --channel release --version 2.1.3 --install-dir C:\Project/.dotnet"] = 0; Actions.RunProcess[@"rm dotnet-install.sh"] = 0; + Actions.RunProcess[@"C:\Project/.dotnet/dotnet --list-runtimes"] = 0; + Actions.RunProcessOut[@"C:\Project/.dotnet/dotnet --list-runtimes"] = @"Microsoft.AspNetCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] +Microsoft.NETCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]"; Actions.RunProcess[@"C:\Project/.dotnet/dotnet --info"] = 0; Actions.RunProcess[@"C:\Project/.dotnet/dotnet clean test.csproj"] = 0; Actions.RunProcess[@"C:\Project/.dotnet/dotnet restore test.csproj"] = 0; - Actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/.dotnet/dotnet build --no-incremental /p:UseSharedCompilation=false test.csproj"] = 0; + Actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/.dotnet/dotnet build --no-incremental test.csproj"] = 0; Actions.RunProcess[@"C:\codeql\tools\java/bin/java -jar C:\codeql\csharp/tools/extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa/tools/odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; @@ -930,18 +947,24 @@ namespace Semmle.Extraction.Tests Actions.LoadXml["test.csproj"] = xml; var autobuilder = CreateAutoBuilder("csharp", false, dotnetVersion: "2.1.3"); - TestAutobuilderScript(autobuilder, 0, 11); + TestAutobuilderScript(autobuilder, 0, 12); } [Fact] public void TestDotnetVersionAlreadyInstalled() { Actions.RunProcess["dotnet --list-sdks"] = 0; - Actions.RunProcessOut["dotnet --list-sdks"] = "2.1.3 [C:\\Program Files\\dotnet\\sdks]\n2.1.4 [C:\\Program Files\\dotnet\\sdks]"; + Actions.RunProcessOut["dotnet --list-sdks"] = @"2.1.3 [C:\Program Files\dotnet\sdks] +2.1.4 [C:\Program Files\dotnet\sdks]"; Actions.RunProcess[@"curl -L -sO https://dot.net/v1/dotnet-install.sh"] = 0; Actions.RunProcess[@"chmod u+x dotnet-install.sh"] = 0; Actions.RunProcess[@"./dotnet-install.sh --channel release --version 2.1.3 --install-dir C:\Project/.dotnet"] = 0; Actions.RunProcess[@"rm dotnet-install.sh"] = 0; + Actions.RunProcess[@"C:\Project/.dotnet/dotnet --list-runtimes"] = 0; + Actions.RunProcessOut[@"C:\Project/.dotnet/dotnet --list-runtimes"] = @"Microsoft.AspNetCore.App 2.1.3 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] +Microsoft.AspNetCore.App 2.1.4 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] +Microsoft.NETCore.App 2.1.3 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] +Microsoft.NETCore.App 2.1.4 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]"; Actions.RunProcess[@"C:\Project/.dotnet/dotnet --info"] = 0; Actions.RunProcess[@"C:\Project/.dotnet/dotnet clean test.csproj"] = 0; Actions.RunProcess[@"C:\Project/.dotnet/dotnet restore test.csproj"] = 0; @@ -966,7 +989,7 @@ namespace Semmle.Extraction.Tests Actions.LoadXml["test.csproj"] = xml; var autobuilder = CreateAutoBuilder("csharp", false, dotnetVersion: "2.1.3"); - TestAutobuilderScript(autobuilder, 0, 11); + TestAutobuilderScript(autobuilder, 0, 12); } [Fact] @@ -979,7 +1002,7 @@ namespace Semmle.Extraction.Tests Actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet --info"] = 0; Actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet clean test.csproj"] = 0; Actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet restore test.csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false test.csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental test.csproj"] = 0; Actions.RunProcess[@"cmd.exe /C C:\codeql\tools\java\bin\java -jar C:\codeql\csharp\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; diff --git a/csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs b/csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs index 001934f489d..73e6fcd181f 100644 --- a/csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs +++ b/csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs @@ -214,8 +214,16 @@ namespace Semmle.Autobuild if (Options.IgnoreErrors) script |= BuildScript.Success; - void startCallback(string s) => Log(Severity.Info, $"\nRunning {s}"); - void exitCallback(int ret, string msg) => Log(Severity.Info, $"Exit code {ret}{(string.IsNullOrEmpty(msg) ? "" : $": {msg}")}"); + void startCallback(string s, bool silent) + { + Log(silent ? Severity.Debug : Severity.Info, $"\nRunning {s}"); + } + + void exitCallback(int ret, string msg, bool silent) + { + Log(silent ? Severity.Debug : Severity.Info, $"Exit code {ret}{(string.IsNullOrEmpty(msg) ? "" : $": {msg}")}"); + } + return script.Run(Actions, startCallback, exitCallback); } diff --git a/csharp/autobuilder/Semmle.Autobuild/BuildActions.cs b/csharp/autobuilder/Semmle.Autobuild/BuildActions.cs index edf4fc752c1..837f6e3f69e 100644 --- a/csharp/autobuilder/Semmle.Autobuild/BuildActions.cs +++ b/csharp/autobuilder/Semmle.Autobuild/BuildActions.cs @@ -141,12 +141,8 @@ namespace Semmle.Autobuild pi.WorkingDirectory = workingDirectory; // Environment variables can only be used when not redirecting stdout - if (!redirectStandardOutput) - { - pi.Environment["UseSharedCompilation"] = "false"; - if (environment != null) - environment.ForEach(kvp => pi.Environment[kvp.Key] = kvp.Value); - } + if (!redirectStandardOutput && environment != null) + environment.ForEach(kvp => pi.Environment[kvp.Key] = kvp.Value); return pi; } diff --git a/csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs b/csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs index 09c41ccf74c..e7e2c9255dd 100644 --- a/csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs +++ b/csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs @@ -43,9 +43,9 @@ namespace Semmle.Autobuild var dir = Path.GetDirectoryName(scriptPath); // A specific .NET Core version may be required - return chmodScript & DotNetRule.WithDotNet(builder, dotNet => + return chmodScript & DotNetRule.WithDotNet(builder, environment => { - var command = new CommandBuilder(builder.Actions, dir, dotNet?.Environment); + var command = new CommandBuilder(builder.Actions, dir, environment); // A specific Visual Studio version may be required var vsTools = MsBuildRule.GetVcVarsBatFile(builder); diff --git a/csharp/autobuilder/Semmle.Autobuild/BuildCommandRule.cs b/csharp/autobuilder/Semmle.Autobuild/BuildCommandRule.cs index 2e582b707f2..fe91503ec8f 100644 --- a/csharp/autobuilder/Semmle.Autobuild/BuildCommandRule.cs +++ b/csharp/autobuilder/Semmle.Autobuild/BuildCommandRule.cs @@ -11,9 +11,9 @@ return BuildScript.Failure; // Custom build commands may require a specific .NET Core version - return DotNetRule.WithDotNet(builder, dotNet => + return DotNetRule.WithDotNet(builder, environment => { - var command = new CommandBuilder(builder.Actions, null, dotNet?.Environment); + var command = new CommandBuilder(builder.Actions, null, environment); // Custom build commands may require a specific Visual Studio version var vsTools = MsBuildRule.GetVcVarsBatFile(builder); diff --git a/csharp/autobuilder/Semmle.Autobuild/BuildScript.cs b/csharp/autobuilder/Semmle.Autobuild/BuildScript.cs index a41011e159f..93ea941d58b 100644 --- a/csharp/autobuilder/Semmle.Autobuild/BuildScript.cs +++ b/csharp/autobuilder/Semmle.Autobuild/BuildScript.cs @@ -25,7 +25,7 @@ namespace Semmle.Autobuild /// an exit message. /// /// The exit code from this build script. - public abstract int Run(IBuildActions actions, Action startCallback, Action exitCallBack); + public abstract int Run(IBuildActions actions, Action startCallback, Action exitCallBack); /// /// Run this build command. @@ -44,33 +44,36 @@ namespace Semmle.Autobuild /// /// Contents of standard out. /// The exit code from this build script. - public abstract int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout); + public abstract int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout); class BuildCommand : BuildScript { readonly string exe, arguments, workingDirectory; readonly IDictionary environment; + readonly bool silent; /// /// Create a simple build command. /// /// The executable to run. /// The arguments to the executable, or null. + /// Whether this command should run silently. /// The working directory (null for current directory). /// Additional environment variables. - public BuildCommand(string exe, string argumentsOpt, string workingDirectory = null, IDictionary environment = null) + public BuildCommand(string exe, string argumentsOpt, bool silent, string workingDirectory = null, IDictionary environment = null) { this.exe = exe; this.arguments = argumentsOpt ?? ""; + this.silent = silent; this.workingDirectory = workingDirectory; this.environment = environment; } public override string ToString() => exe + " " + arguments; - public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack) + public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack) { - startCallback(this.ToString()); + startCallback(this.ToString(), silent); var ret = 1; var retMessage = ""; try @@ -83,13 +86,13 @@ namespace Semmle.Autobuild retMessage = ex.Message; } - exitCallBack(ret, retMessage); + exitCallBack(ret, retMessage, silent); return ret; } - public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout) + public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout) { - startCallback(this.ToString()); + startCallback(this.ToString(), silent); var ret = 1; var retMessage = ""; try @@ -102,7 +105,7 @@ namespace Semmle.Autobuild retMessage = ex.Message; stdout = new string[0]; } - exitCallBack(ret, retMessage); + exitCallBack(ret, retMessage, silent); return ret; } @@ -116,9 +119,9 @@ namespace Semmle.Autobuild this.func = func; } - public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack) => func(actions); + public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack) => func(actions); - public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout) + public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout) { stdout = new string[0]; return func(actions); @@ -142,7 +145,7 @@ namespace Semmle.Autobuild this.s2b = s2; } - public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack) + public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack) { int ret1; if (s2a != null) @@ -155,7 +158,7 @@ namespace Semmle.Autobuild return s2b(ret1).Run(actions, startCallback, exitCallBack); } - public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout) + public override int Run(IBuildActions actions, Action startCallback, Action exitCallBack, out IList stdout) { var ret1 = s1.Run(actions, startCallback, exitCallBack, out var stdout1); var ret2 = (s2a != null ? s2a(stdout1, ret1) : s2b(ret1)).Run(actions, startCallback, exitCallBack, out var stdout2); @@ -171,10 +174,11 @@ namespace Semmle.Autobuild /// Creates a simple build script that runs the specified exe. /// /// The arguments to the executable, or null. + /// Whether the executable should run silently. /// The working directory (null for current directory). /// Additional environment variables. - public static BuildScript Create(string exe, string argumentsOpt, string workingDirectory, IDictionary environment) => - new BuildCommand(exe, argumentsOpt, workingDirectory, environment); + public static BuildScript Create(string exe, string argumentsOpt, bool silent, string workingDirectory, IDictionary environment) => + new BuildCommand(exe, argumentsOpt, silent, workingDirectory, environment); /// /// Creates a simple build script that runs the specified function. diff --git a/csharp/autobuilder/Semmle.Autobuild/CommandBuilder.cs b/csharp/autobuilder/Semmle.Autobuild/CommandBuilder.cs index 273cc52b036..7b95e495697 100644 --- a/csharp/autobuilder/Semmle.Autobuild/CommandBuilder.cs +++ b/csharp/autobuilder/Semmle.Autobuild/CommandBuilder.cs @@ -17,13 +17,15 @@ namespace Semmle.Autobuild readonly EscapeMode escapingMode; readonly string workingDirectory; readonly IDictionary environment; + readonly bool silent; /// /// Initializes a new instance of the class. /// /// The working directory (null for current directory). /// Additional environment variables. - public CommandBuilder(IBuildActions actions, string workingDirectory = null, IDictionary environment = null) + /// Whether this command should be run silently. + public CommandBuilder(IBuildActions actions, string workingDirectory = null, IDictionary environment = null, bool silent = false) { arguments = new StringBuilder(); if (actions.IsWindows()) @@ -40,6 +42,7 @@ namespace Semmle.Autobuild firstCommand = true; this.workingDirectory = workingDirectory; this.environment = environment; + this.silent = silent; } void OdasaIndex(string odasa) @@ -190,6 +193,6 @@ namespace Semmle.Autobuild /// /// Returns a build script that contains just this command. /// - public BuildScript Script => BuildScript.Create(executable, arguments.ToString(), workingDirectory, environment); + public BuildScript Script => BuildScript.Create(executable, arguments.ToString(), silent, workingDirectory, environment); } } diff --git a/csharp/autobuilder/Semmle.Autobuild/DotNetRule.cs b/csharp/autobuilder/Semmle.Autobuild/DotNetRule.cs index 789d68db30a..c9b33cef8d1 100644 --- a/csharp/autobuilder/Semmle.Autobuild/DotNetRule.cs +++ b/csharp/autobuilder/Semmle.Autobuild/DotNetRule.cs @@ -5,6 +5,7 @@ using Newtonsoft.Json.Linq; using System.Collections.Generic; using System.IO; using Semmle.Util; +using System.Text.RegularExpressions; namespace Semmle.Autobuild { @@ -34,22 +35,20 @@ namespace Semmle.Autobuild builder.Log(Severity.Info, "Attempting to build using .NET Core"); } - return WithDotNet(builder, dotNet => + return WithDotNet(builder, (dotNetPath, environment, compatibleClr) => { - var ret = GetInfoCommand(builder.Actions, dotNet); + var ret = GetInfoCommand(builder.Actions, dotNetPath, environment); foreach (var projectOrSolution in builder.ProjectsOrSolutionsToBuild) { - var cleanCommand = GetCleanCommand(builder.Actions, dotNet); + var cleanCommand = GetCleanCommand(builder.Actions, dotNetPath, environment); cleanCommand.QuoteArgument(projectOrSolution.FullPath); var clean = cleanCommand.Script; - var restoreCommand = GetRestoreCommand(builder.Actions, dotNet); + var restoreCommand = GetRestoreCommand(builder.Actions, dotNetPath, environment); restoreCommand.QuoteArgument(projectOrSolution.FullPath); var restore = restoreCommand.Script; - var buildCommand = GetBuildCommand(builder, dotNet); - buildCommand.QuoteArgument(projectOrSolution.FullPath); - var build = buildCommand.Script; + var build = GetBuildScript(builder, dotNetPath, environment, compatibleClr, projectOrSolution.FullPath); ret &= clean & BuildScript.Try(restore) & build; } @@ -57,38 +56,73 @@ namespace Semmle.Autobuild }); } - /// - /// Returns a script that attempts to download relevant version(s) of the - /// .NET Core SDK, followed by running the script generated by . - /// - /// The first element DotNetPath of the argument to - /// is the path where .NET Core was installed, and the second element Environment - /// is any additional required environment variables. The tuple argument is null - /// when the installation failed. - /// - public static BuildScript WithDotNet(Autobuilder builder, Func<(string DotNetPath, IDictionary Environment)?, BuildScript> f) + static BuildScript WithDotNet(Autobuilder builder, Func, bool, BuildScript> f) { var installDir = builder.Actions.PathCombine(builder.Options.RootDirectory, ".dotnet"); var installScript = DownloadDotNet(builder, installDir); return BuildScript.Bind(installScript, installed => + { + Dictionary env; + if (installed == 0) { - if (installed == 0) - { - // The installation succeeded, so use the newly installed .NET Core - var path = builder.Actions.GetEnvironmentVariable("PATH"); - var delim = builder.Actions.IsWindows() ? ";" : ":"; - var env = new Dictionary{ + // The installation succeeded, so use the newly installed .NET Core + var path = builder.Actions.GetEnvironmentVariable("PATH"); + var delim = builder.Actions.IsWindows() ? ";" : ":"; + env = new Dictionary{ { "DOTNET_MULTILEVEL_LOOKUP", "false" }, // prevent look up of other .NET Core SDKs { "DOTNET_SKIP_FIRST_TIME_EXPERIENCE", "true" }, { "PATH", installDir + delim + path } }; - return f((installDir, env)); + } + else + { + installDir = null; + env = null; + } + + // The CLR tracer is always compatible on Windows + if (builder.Actions.IsWindows()) + return f(installDir, env, true); + + // The CLR tracer is only compatible on .NET Core >= 3 on Linux and macOS (see + // https://github.com/dotnet/coreclr/issues/19622) + return BuildScript.Bind(GetInstalledRuntimesScript(builder.Actions, installDir, env), (runtimes, runtimesRet) => + { + var compatibleClr = false; + if (runtimesRet == 0) + { + var minimumVersion = new Version(3, 0); + var regex = new Regex(@"Microsoft\.NETCore\.App (\d\.\d\.\d)"); + compatibleClr = runtimes. + Select(runtime => regex.Match(runtime)). + Where(m => m.Success). + Select(m => m.Groups[1].Value). + Any(m => Version.TryParse(m, out var v) && v >= minimumVersion); } - return f(null); + if (!compatibleClr) + { + if (env == null) + env = new Dictionary(); + env.Add("UseSharedCompilation", "false"); + } + + return f(installDir, env, compatibleClr); }); + }); } + /// + /// Returns a script that attempts to download relevant version(s) of the + /// .NET Core SDK, followed by running the script generated by . + /// + /// The argument to is any additional required environment + /// variables needed by the installed .NET Core (null when no variables + /// are needed). + /// + public static BuildScript WithDotNet(Autobuilder builder, Func, BuildScript> f) + => WithDotNet(builder, (_1, env, _2) => f(env)); + /// /// Returns a script for downloading relevant versions of the /// .NET Core SDK. The SDK(s) will be installed at installDir @@ -225,7 +259,7 @@ Invoke-Command -ScriptBlock $ScriptBlock"; static BuildScript GetInstalledSdksScript(IBuildActions actions) { - var listSdks = new CommandBuilder(actions). + var listSdks = new CommandBuilder(actions, silent: true). RunCommand("dotnet"). Argument("--list-sdks"); return listSdks.Script; @@ -234,38 +268,62 @@ Invoke-Command -ScriptBlock $ScriptBlock"; static string DotNetCommand(IBuildActions actions, string dotNetPath) => dotNetPath != null ? actions.PathCombine(dotNetPath, "dotnet") : "dotnet"; - BuildScript GetInfoCommand(IBuildActions actions, (string DotNetPath, IDictionary Environment)? arg) + BuildScript GetInfoCommand(IBuildActions actions, string dotNetPath, IDictionary environment) { - var info = new CommandBuilder(actions, null, arg?.Environment). - RunCommand(DotNetCommand(actions, arg?.DotNetPath)). + var info = new CommandBuilder(actions, null, environment). + RunCommand(DotNetCommand(actions, dotNetPath)). Argument("--info"); return info.Script; } - CommandBuilder GetCleanCommand(IBuildActions actions, (string DotNetPath, IDictionary Environment)? arg) + CommandBuilder GetCleanCommand(IBuildActions actions, string dotNetPath, IDictionary environment) { - var clean = new CommandBuilder(actions, null, arg?.Environment). - RunCommand(DotNetCommand(actions, arg?.DotNetPath)). + var clean = new CommandBuilder(actions, null, environment). + RunCommand(DotNetCommand(actions, dotNetPath)). Argument("clean"); return clean; } - CommandBuilder GetRestoreCommand(IBuildActions actions, (string DotNetPath, IDictionary Environment)? arg) + CommandBuilder GetRestoreCommand(IBuildActions actions, string dotNetPath, IDictionary environment) { - var restore = new CommandBuilder(actions, null, arg?.Environment). - RunCommand(DotNetCommand(actions, arg?.DotNetPath)). + var restore = new CommandBuilder(actions, null, environment). + RunCommand(DotNetCommand(actions, dotNetPath)). Argument("restore"); return restore; } - CommandBuilder GetBuildCommand(Autobuilder builder, (string DotNetPath, IDictionary Environment)? arg) + static BuildScript GetInstalledRuntimesScript(IBuildActions actions, string dotNetPath, IDictionary environment) { - var build = new CommandBuilder(builder.Actions, null, arg?.Environment); - return builder.MaybeIndex(build, DotNetCommand(builder.Actions, arg?.DotNetPath)). + var listSdks = new CommandBuilder(actions, environment: environment, silent: true). + RunCommand(DotNetCommand(actions, dotNetPath)). + Argument("--list-runtimes"); + return listSdks.Script; + } + + /// + /// Gets the `dotnet build` script. + /// + /// The CLR tracer only works on .NET Core >= 3 on Linux and macOS (see + /// https://github.com/dotnet/coreclr/issues/19622), so in case we are + /// running on an older .NET Core, we disable shared compilation (and + /// hence the need for CLR tracing), by adding a + /// `/p:UseSharedCompilation=false` argument. + /// + BuildScript GetBuildScript(Autobuilder builder, string dotNetPath, IDictionary environment, bool compatibleClr, string projOrSln) + { + var build = new CommandBuilder(builder.Actions, null, environment); + var script = builder.MaybeIndex(build, DotNetCommand(builder.Actions, dotNetPath)). Argument("build"). - Argument("--no-incremental"). - Argument("/p:UseSharedCompilation=false"). - Argument(builder.Options.DotNetArguments); + Argument("--no-incremental"); + + return compatibleClr ? + script.Argument(builder.Options.DotNetArguments). + QuoteArgument(projOrSln). + Script : + script.Argument("/p:UseSharedCompilation=false"). + Argument(builder.Options.DotNetArguments). + QuoteArgument(projOrSln). + Script; } } } diff --git a/csharp/ql/src/Security Features/CWE-016/ASPNetMaxRequestLength.qhelp b/csharp/ql/src/Security Features/CWE-016/ASPNetMaxRequestLength.qhelp new file mode 100644 index 00000000000..2d9454c16d5 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-016/ASPNetMaxRequestLength.qhelp @@ -0,0 +1,47 @@ + + + + +

+ The maxRequestLength attribute sets the limit for the input + stream buffering threshold in KB. Attackers can use large requests to cause + denial-of-service attacks. +

+
+ + +

+ The recommended value is 4096 KB but you should try setting it as + small as possible according to business requirements. +

+ +
+ + +

+ The following example shows the maxRequestLength + attribute set to a high value (255 MB) in a Web.config + file for ASP.NET: +

+ + + +

+ Unless such a high value is strictly needed, it is better to set + the recommended value (4096 KB): +

+ + + +
+ + + +
  • + MSDN: + HttpRuntimeSection.MaxRequestLength Property. +
  • +
    +
    \ No newline at end of file diff --git a/csharp/ql/src/Security Features/CWE-016/ASPNetMaxRequestLength.ql b/csharp/ql/src/Security Features/CWE-016/ASPNetMaxRequestLength.ql new file mode 100644 index 00000000000..dbf1a527f2c --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-016/ASPNetMaxRequestLength.ql @@ -0,0 +1,22 @@ +/** + * @name Large 'maxRequestLength' value + * @description Setting a large 'maxRequestLength' value may render a webpage vulnerable to + * denial-of-service attacks. + * @kind problem + * @problem.severity warning + * @id cs/web/large-max-request-length + * @tags security + * frameworks/asp.net + * external/cwe/cwe-16 + */ + +import csharp +import semmle.code.asp.WebConfig + +from SystemWebXMLElement web, XMLAttribute maxReqLength +where + maxReqLength = web + .getAChild(any(string s | s.toLowerCase() = "httpruntime")) + .getAttribute(any(string s | s.toLowerCase() = "maxrequestlength")) and + maxReqLength.getValue().toInt() > 4096 +select maxReqLength, "Large 'maxRequestLength' value (" + maxReqLength.getValue() + " KB)." diff --git a/csharp/ql/src/Security Features/CWE-016/Web.config.ASPNetMaxRequestLength.bad b/csharp/ql/src/Security Features/CWE-016/Web.config.ASPNetMaxRequestLength.bad new file mode 100644 index 00000000000..266d77385c1 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-016/Web.config.ASPNetMaxRequestLength.bad @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/csharp/ql/src/Security Features/CWE-016/Web.config.ASPNetMaxRequestLength.good b/csharp/ql/src/Security Features/CWE-016/Web.config.ASPNetMaxRequestLength.good new file mode 100644 index 00000000000..8ba76b53dae --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-016/Web.config.ASPNetMaxRequestLength.good @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll index 7ecb474f632..9519a74265f 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll index 7ecb474f632..9519a74265f 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll index 7ecb474f632..9519a74265f 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll index 7ecb474f632..9519a74265f 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll index 7ecb474f632..9519a74265f 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll index 8b9c8f5abd3..91f5aeacc9d 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll @@ -57,14 +57,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -95,7 +95,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -183,7 +183,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -191,7 +191,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -226,7 +226,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -260,7 +260,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlowNoCtx(p, mid) and localValueStep(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -296,8 +296,8 @@ private module ImplCommon { setterCall(call, i1, i2, f) and node1.(ArgumentNode).argumentOf(call, i1) and node2.getPreUpdateNode().(ArgumentNode).argumentOf(call, i2) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -333,8 +333,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | storeReturn0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -365,8 +365,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | read0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getContainerType()) and - compatibleTypes(node2.getTypeBound(), f.getType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getContainerType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getType()) ) } @@ -384,7 +384,7 @@ private module ImplCommon { store(node1, f, mid1) and localValueStep*(mid1, mid2) and read(mid2, f, node2) and - compatibleTypes(node1.getTypeBound(), node2.getTypeBound()) + compatibleTypes(getErasedNodeTypeBound(node1), getErasedNodeTypeBound(node2)) ) } @@ -405,14 +405,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -443,7 +443,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -531,7 +531,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -539,7 +539,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -574,7 +574,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -860,4 +860,10 @@ private module ImplCommon { or result = viableCallable(call) and cc instanceof CallContextReturn } + + pragma[noinline] + DataFlowType getErasedNodeType(Node n) { result = getErasedRepr(n.getType()) } + + pragma[noinline] + DataFlowType getErasedNodeTypeBound(Node n) { result = getErasedRepr(n.getTypeBound()) } } diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index d30ed00a0c4..9df3803fa3d 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -1307,7 +1307,7 @@ private predicate suppressUnusedType(DotNet::Type t) { any() } * Type-based pruning is disabled for now, so this is a stub implementation. */ bindingset[t] -DotNet::Type getErasedRepr(DotNet::Type t) { +DataFlowType getErasedRepr(DotNet::Type t) { // stub implementation suppressUnusedType(t) and result instanceof ObjectType } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll index e10e266b2ab..f9efad42fb0 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRVariable.qll @@ -13,14 +13,20 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var } /** - * Represents a variable referenced by the IR for a function. The variable may - * be a user-declared variable (`IRUserVariable`) or a temporary variable - * generated by the AST-to-IR translation (`IRTempVariable`). + * A variable referenced by the IR for a function. The variable may be a user-declared variable + * (`IRUserVariable`) or a temporary variable generated by the AST-to-IR translation + * (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + IRVariable() { + this = TIRUserVariable(_, _, func) or + this = TIRTempVariable(func, _, _, _) or + this = TIRStringLiteral(func, _, _, _) + } + + string toString() { none() } /** * Holds if this variable's value cannot be changed within a function. Currently used for string @@ -41,19 +47,19 @@ abstract class IRVariable extends TIRVariable { /** * Gets the type of the variable. */ - abstract Language::LanguageType getLanguageType(); + Language::LanguageType getLanguageType() { none() } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { none() } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { none() } /** * Gets the source location of this variable. @@ -72,7 +78,7 @@ abstract class IRVariable extends TIRVariable { } /** - * Represents a user-declared variable referenced by the IR for a function. + * A user-declared variable referenced by the IR for a function. */ class IRUserVariable extends IRVariable, TIRUserVariable { Language::Variable var; @@ -97,20 +103,34 @@ class IRUserVariable extends IRVariable, TIRUserVariable { } /** - * Represents a variable (user-declared or temporary) that is allocated on the - * stack. This includes all parameters, non-static local variables, and - * temporary variables. + * A variable (user-declared or temporary) that is allocated on the stack. This includes all + * parameters, non-static local variables, and temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { } +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + exists(Language::Variable var | + this = TIRUserVariable(var, _, func) and + Language::isVariableAutomatic(var) + ) + or + this = TIRTempVariable(func, _, _, _) + } +} +/** + * A user-declared variable that is allocated on the stack. This includes all parameters and + * non-static local variables. + */ class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { override Language::AutomaticVariable var; - IRAutomaticUserVariable() { Language::isVariableAutomatic(var) } - final override Language::AutomaticVariable getVariable() { result = var } } +/** + * A user-declared variable that is not allocated on the stack. This includes all global variables, + * namespace-scope variables, static fields, and static local variables. + */ class IRStaticUserVariable extends IRUserVariable { override Language::StaticVariable var; @@ -119,10 +139,19 @@ class IRStaticUserVariable extends IRUserVariable { final override Language::StaticVariable getVariable() { result = var } } -abstract class IRGeneratedVariable extends IRVariable { +/** + * A variable that is not user-declared. This includes temporary variables generated as part of IR + * construction, as well as string literals. + */ +class IRGeneratedVariable extends IRVariable { Language::AST ast; Language::LanguageType type; + IRGeneratedVariable() { + this = TIRTempVariable(func, ast, _, type) or + this = TIRStringLiteral(func, ast, type, _) + } + final override Language::LanguageType getLanguageType() { result = type } final override Language::AST getAST() { result = ast } @@ -144,6 +173,11 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } +/** + * A temporary variable introduced by IR construction. The most common examples are the variable + * generated to hold the return value of afunction, or the variable generated to hold the result of + * a condition operator (`a ? b : c`). + */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { TempVariableTag tag; @@ -158,18 +192,28 @@ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVa override string getBaseString() { result = "#temp" } } +/** + * A temporary variable generated to hold the return value of a function. + */ class IRReturnVariable extends IRTempVariable { IRReturnVariable() { tag = ReturnValueTempVar() } final override string toString() { result = "#return" } } +/** + * A temporary variable generated to hold the exception thrown by a `ThrowValue` instruction. + */ class IRThrowVariable extends IRTempVariable { IRThrowVariable() { tag = ThrowTempVar() } override string getBaseString() { result = "#throw" } } +/** + * A variable generated to represent the contents of a string literal. This variable acts much like + * a read-only global variable. + */ class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral { Language::StringLiteral literal; diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll index e3c78f476e9..8819826d5c3 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll @@ -94,12 +94,21 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand( + Instruction instr, string message, IRFunction func, string funcText + ) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll index e10e266b2ab..f9efad42fb0 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRVariable.qll @@ -13,14 +13,20 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var } /** - * Represents a variable referenced by the IR for a function. The variable may - * be a user-declared variable (`IRUserVariable`) or a temporary variable - * generated by the AST-to-IR translation (`IRTempVariable`). + * A variable referenced by the IR for a function. The variable may be a user-declared variable + * (`IRUserVariable`) or a temporary variable generated by the AST-to-IR translation + * (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + IRVariable() { + this = TIRUserVariable(_, _, func) or + this = TIRTempVariable(func, _, _, _) or + this = TIRStringLiteral(func, _, _, _) + } + + string toString() { none() } /** * Holds if this variable's value cannot be changed within a function. Currently used for string @@ -41,19 +47,19 @@ abstract class IRVariable extends TIRVariable { /** * Gets the type of the variable. */ - abstract Language::LanguageType getLanguageType(); + Language::LanguageType getLanguageType() { none() } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { none() } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { none() } /** * Gets the source location of this variable. @@ -72,7 +78,7 @@ abstract class IRVariable extends TIRVariable { } /** - * Represents a user-declared variable referenced by the IR for a function. + * A user-declared variable referenced by the IR for a function. */ class IRUserVariable extends IRVariable, TIRUserVariable { Language::Variable var; @@ -97,20 +103,34 @@ class IRUserVariable extends IRVariable, TIRUserVariable { } /** - * Represents a variable (user-declared or temporary) that is allocated on the - * stack. This includes all parameters, non-static local variables, and - * temporary variables. + * A variable (user-declared or temporary) that is allocated on the stack. This includes all + * parameters, non-static local variables, and temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { } +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + exists(Language::Variable var | + this = TIRUserVariable(var, _, func) and + Language::isVariableAutomatic(var) + ) + or + this = TIRTempVariable(func, _, _, _) + } +} +/** + * A user-declared variable that is allocated on the stack. This includes all parameters and + * non-static local variables. + */ class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { override Language::AutomaticVariable var; - IRAutomaticUserVariable() { Language::isVariableAutomatic(var) } - final override Language::AutomaticVariable getVariable() { result = var } } +/** + * A user-declared variable that is not allocated on the stack. This includes all global variables, + * namespace-scope variables, static fields, and static local variables. + */ class IRStaticUserVariable extends IRUserVariable { override Language::StaticVariable var; @@ -119,10 +139,19 @@ class IRStaticUserVariable extends IRUserVariable { final override Language::StaticVariable getVariable() { result = var } } -abstract class IRGeneratedVariable extends IRVariable { +/** + * A variable that is not user-declared. This includes temporary variables generated as part of IR + * construction, as well as string literals. + */ +class IRGeneratedVariable extends IRVariable { Language::AST ast; Language::LanguageType type; + IRGeneratedVariable() { + this = TIRTempVariable(func, ast, _, type) or + this = TIRStringLiteral(func, ast, type, _) + } + final override Language::LanguageType getLanguageType() { result = type } final override Language::AST getAST() { result = ast } @@ -144,6 +173,11 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } +/** + * A temporary variable introduced by IR construction. The most common examples are the variable + * generated to hold the return value of afunction, or the variable generated to hold the result of + * a condition operator (`a ? b : c`). + */ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable { TempVariableTag tag; @@ -158,18 +192,28 @@ class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVa override string getBaseString() { result = "#temp" } } +/** + * A temporary variable generated to hold the return value of a function. + */ class IRReturnVariable extends IRTempVariable { IRReturnVariable() { tag = ReturnValueTempVar() } final override string toString() { result = "#return" } } +/** + * A temporary variable generated to hold the exception thrown by a `ThrowValue` instruction. + */ class IRThrowVariable extends IRTempVariable { IRThrowVariable() { tag = ThrowTempVar() } override string getBaseString() { result = "#throw" } } +/** + * A variable generated to represent the contents of a string literal. This variable acts much like + * a read-only global variable. + */ class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral { Language::StringLiteral literal; diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll index e3c78f476e9..8819826d5c3 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll @@ -94,12 +94,21 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand( + Instruction instr, string message, IRFunction func, string funcText + ) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll index a789edc7590..96b4c28db82 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll @@ -865,3 +865,17 @@ private module CachedForDebugging { result.getTag() = var.getTag() } } + +module SSASanity { + query predicate multipleOperandMemoryLocations( + OldIR::MemoryOperand operand, string message, OldIR::IRFunction func, string funcText + ) { + exists(int locationCount | + locationCount = strictcount(Alias::getOperandMemoryLocation(operand)) and + locationCount > 1 and + func = operand.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) and + message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'." + ) + } +} diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.ql b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.ql new file mode 100644 index 00000000000..43f303dd024 --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.ql @@ -0,0 +1,8 @@ +/** + * @name Unaliased SSA Sanity Check + * @description Performs sanity checks on the SSA construction. This query should have no results. + * @kind table + * @id csharp/unaliased-ssa-sanity-check + */ + +import SSASanity diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.qll new file mode 100644 index 00000000000..95e8443b2a3 --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.qll @@ -0,0 +1,2 @@ +private import SSAConstruction as SSA +import SSA::SSASanity diff --git a/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerConstant.qll b/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerConstant.qll index 55546ba380f..6034ebc5674 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerConstant.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerConstant.qll @@ -192,3 +192,33 @@ predicate isGT(IntValue a, IntValue b) { hasValue(a) and hasValue(b) and a > b } */ bindingset[a, b] predicate isGE(IntValue a, IntValue b) { hasValue(a) and hasValue(b) and a >= b } + +/** + * Converts the bit count in `bits` to a byte count and a bit count in the form + * "bytes:bits". If `bits` represents an integer number of bytes, the ":bits" section is omitted. + * If `bits` does not have a known value, the result is "?". + */ +bindingset[bits] +string bitsToBytesAndBits(IntValue bits) { + exists(int bytes, int leftoverBits | + hasValue(bits) and + bytes = bits / 8 and + leftoverBits = bits % 8 and + if leftoverBits = 0 then result = bytes.toString() else result = bytes + ":" + leftoverBits + ) + or + not hasValue(bits) and result = "?" +} + +/** + * Gets a printable string for a bit offset with possibly unknown value. + */ +bindingset[bitOffset] +string getBitOffsetString(IntValue bitOffset) { + if hasValue(bitOffset) + then + if bitOffset >= 0 + then result = "+" + bitsToBytesAndBits(bitOffset) + else result = "-" + bitsToBytesAndBits(neg(bitOffset)) + else result = "+?" +} diff --git a/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerInterval.qll b/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerInterval.qll index bc09f9c2243..cd12b9b627a 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerInterval.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/internal/IntegerInterval.qll @@ -30,5 +30,5 @@ Overlap getOverlap(IntValue defStart, IntValue defEnd, IntValue useStart, IntVal bindingset[start, end] string getIntervalString(IntValue start, IntValue end) { // We represent an interval has half-open, so print it as "[start..end)". - result = "[" + intValueToString(start) + ".." + intValueToString(end) + ")" + result = "[" + bitsToBytesAndBits(start) + ".." + bitsToBytesAndBits(end) + ")" } diff --git a/csharp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected b/csharp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/csharp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref b/csharp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref new file mode 100644 index 00000000000..1b7d4a7996a --- /dev/null +++ b/csharp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.cs b/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.cs new file mode 100644 index 00000000000..a3e588b5c5b --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.cs @@ -0,0 +1,5 @@ +// Dummy class for extraction purposes +public class ASPNetMaxRequestLengthDummyClass +{ + +} \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.expected b/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.expected new file mode 100644 index 00000000000..1feab65d0ee --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.expected @@ -0,0 +1 @@ +| bad/Web.config:4:5:4:46 | maxRequestLength=262144 | Large 'maxRequestLength' value (262144 KB). | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.qlref b/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.qlref new file mode 100644 index 00000000000..7469d5d2e98 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.qlref @@ -0,0 +1 @@ +Security Features/CWE-016/ASPNetMaxRequestLength.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-016/Web.config b/csharp/ql/test/query-tests/Security Features/CWE-016/Web.config new file mode 100644 index 00000000000..8ba76b53dae --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-016/Web.config @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-016/bad/Web.config b/csharp/ql/test/query-tests/Security Features/CWE-016/bad/Web.config new file mode 100644 index 00000000000..ea52bd2505b --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-016/bad/Web.config @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/docs/query-metadata-style-guide.md b/docs/query-metadata-style-guide.md index d5f5606002c..70cb083cb1a 100644 --- a/docs/query-metadata-style-guide.md +++ b/docs/query-metadata-style-guide.md @@ -113,7 +113,7 @@ Alert queries (`@kind problem` or `path-problem`) support two further properties * `low ` * `medium ` * `high ` - * `very high` + * `very-high` * `@problem.severity`–defines the level of severity of the alert: * `error`–an issue that is likely to cause incorrect program behavior, for example a crash or vulnerability. * `warning`–an issue that indicates a potential problem in the code, or makes the code fragile if another (unrelated) part of code is changed. diff --git a/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql b/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql index 8774de339b6..353df33afed 100644 --- a/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql +++ b/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql @@ -11,7 +11,7 @@ * external/cwe/cwe-088 */ -import semmle.code.java.Expr +import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.ExternalProcess import ExecCommon diff --git a/java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql b/java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql index e80afa7f6eb..a0fa793872f 100644 --- a/java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql +++ b/java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql @@ -11,7 +11,7 @@ * external/cwe/cwe-088 */ -import semmle.code.java.Expr +import java import semmle.code.java.security.ExternalProcess import ExecCommon diff --git a/java/ql/src/Security/CWE/CWE-089/SqlTainted.ql b/java/ql/src/Security/CWE/CWE-089/SqlTainted.ql index 5b79c56a83c..5750829d7d6 100644 --- a/java/ql/src/Security/CWE/CWE-089/SqlTainted.ql +++ b/java/ql/src/Security/CWE/CWE-089/SqlTainted.ql @@ -10,7 +10,7 @@ * external/cwe/cwe-089 */ -import semmle.code.java.Expr +import java import semmle.code.java.dataflow.FlowSources import SqlInjectionLib import DataFlow::PathGraph diff --git a/java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql b/java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql index 83eaa50ea2c..e721cb7fba6 100644 --- a/java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql +++ b/java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql @@ -10,6 +10,7 @@ * external/cwe/cwe-089 */ +import java import semmle.code.java.security.SqlUnescapedLib import SqlInjectionLib diff --git a/java/ql/src/semmle/code/java/dataflow/NullGuards.qll b/java/ql/src/semmle/code/java/dataflow/NullGuards.qll index 43425f2f908..7069699eb5d 100644 --- a/java/ql/src/semmle/code/java/dataflow/NullGuards.qll +++ b/java/ql/src/semmle/code/java/dataflow/NullGuards.qll @@ -24,6 +24,21 @@ Expr enumConstEquality(Expr e, boolean polarity, EnumConstant c) { ) } +/** Gets an instanceof expression of `v` with type `type` */ +InstanceOfExpr instanceofExpr(SsaVariable v, Type type) { + result.getTypeName().getType() = type and + result.getExpr() = v.getAUse() +} + +/** + * Gets an expression of the form `v1 == v2` or `v1 != v2`. + * The predicate is symmetric in `v1` and `v2`. + */ +EqualityTest varEqualityTestExpr(SsaVariable v1, SsaVariable v2, boolean isEqualExpr) { + result.hasOperands(v1.getAUse(), v2.getAUse()) and + isEqualExpr = result.polarity() +} + /** Gets an expression that is provably not `null`. */ Expr clearlyNotNullExpr(Expr reason) { result instanceof ClassInstanceExpr and reason = result diff --git a/java/ql/src/semmle/code/java/dataflow/Nullness.qll b/java/ql/src/semmle/code/java/dataflow/Nullness.qll index d7da85832ae..c3faeaf0ff2 100644 --- a/java/ql/src/semmle/code/java/dataflow/Nullness.qll +++ b/java/ql/src/semmle/code/java/dataflow/Nullness.qll @@ -515,6 +515,18 @@ private predicate correlatedConditions( cond2.getCondition() = enumConstEquality(v.getAUse(), pol2, c) and inverted = pol1.booleanXor(pol2) ) + or + exists(SsaVariable v, Type type | + cond1.getCondition() = instanceofExpr(v, type) and + cond2.getCondition() = instanceofExpr(v, type) and + inverted = false + ) + or + exists(SsaVariable v1, SsaVariable v2, boolean branch1, boolean branch2 | + cond1.getCondition() = varEqualityTestExpr(v1, v2, branch1) and + cond2.getCondition() = varEqualityTestExpr(v1, v2, branch2) and + inverted = branch1.booleanXor(branch2) + ) ) } diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll index 7ecb474f632..9519a74265f 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll index 7ecb474f632..9519a74265f 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll index 7ecb474f632..9519a74265f 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll index 7ecb474f632..9519a74265f 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll index 7ecb474f632..9519a74265f 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll @@ -464,7 +464,7 @@ private predicate simpleParameterFlow( ) { throughFlowNodeCand(node, config) and p = node and - t = getErasedRepr(node.getType()) and + t = getErasedNodeType(node) and exists(ReturnNode ret, ReturnKind kind | returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and kind = ret.getKind() and @@ -475,21 +475,21 @@ private predicate simpleParameterFlow( exists(Node mid | simpleParameterFlow(p, mid, t, config) and localFlowStep(mid, node, config) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, _, config) and additionalLocalFlowStep(mid, node, config) and - t = getErasedRepr(node.getType()) + t = getErasedNodeType(node) ) or throughFlowNodeCand(node, unbind(config)) and exists(Node mid | simpleParameterFlow(p, mid, t, config) and localStoreReadStep(mid, node) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // value flow through a callable @@ -497,7 +497,7 @@ private predicate simpleParameterFlow( exists(Node arg | simpleParameterFlow(p, arg, t, config) and argumentValueFlowsThrough(arg, node, _) and - compatibleTypes(t, node.getType()) + compatibleTypes(t, getErasedNodeType(node)) ) or // flow through a callable @@ -989,7 +989,9 @@ private class CastingNode extends Node { */ private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) { flowCandFwd0(node, fromArg, apf, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), apf.getType()) + else any() } /** @@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node { } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path front for this node. */ AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) } @@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node { AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) } pragma[noinline] - private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) } + private DataFlowType getErasedReprType() { result = getErasedNodeType(this) } /** Gets the `nil` path for this node. */ AccessPathNil getAp() { result = TNil(this.getErasedReprType()) } @@ -2076,7 +2078,7 @@ private module FlowExploration { TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) { config.isSource(node) and cc instanceof CallContextAny and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2091,7 +2093,9 @@ private module FlowExploration { exists(PartialPathNode mid | partialPathStep(mid, node, cc, ap, config) and not fullBarrier(node, config) and - if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any() + if node instanceof CastingNode + then compatibleTypes(getErasedNodeType(node), ap.getType()) + else any() ) } @@ -2194,7 +2198,7 @@ private module FlowExploration { additionalLocalFlowStep(mid.getNode(), node, config) and cc = mid.getCallContext() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() ) or @@ -2206,7 +2210,7 @@ private module FlowExploration { additionalJumpStep(mid.getNode(), node, config) and cc instanceof CallContextAny and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedRepr(node.getType())) and + ap = TPartialNil(getErasedNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll index 8b9c8f5abd3..91f5aeacc9d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -57,14 +57,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -95,7 +95,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -183,7 +183,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -191,7 +191,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -226,7 +226,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -260,7 +260,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlowNoCtx(p, mid) and localValueStep(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -296,8 +296,8 @@ private module ImplCommon { setterCall(call, i1, i2, f) and node1.(ArgumentNode).argumentOf(call, i1) and node2.getPreUpdateNode().(ArgumentNode).argumentOf(call, i2) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -333,8 +333,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | storeReturn0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getType()) and - compatibleTypes(node2.getTypeBound(), f.getContainerType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getContainerType()) ) } @@ -365,8 +365,8 @@ private module ImplCommon { exists(DataFlowCall call, ReturnKind kind | read0(call, kind, node1, f) and node2 = getAnOutNode(call, kind) and - compatibleTypes(node1.getTypeBound(), f.getContainerType()) and - compatibleTypes(node2.getTypeBound(), f.getType()) + compatibleTypes(getErasedNodeTypeBound(node1), f.getContainerType()) and + compatibleTypes(getErasedNodeTypeBound(node2), f.getType()) ) } @@ -384,7 +384,7 @@ private module ImplCommon { store(node1, f, mid1) and localValueStep*(mid1, mid2) and read(mid2, f, node2) and - compatibleTypes(node1.getTypeBound(), node2.getTypeBound()) + compatibleTypes(getErasedNodeTypeBound(node1), getErasedNodeTypeBound(node2)) ) } @@ -405,14 +405,14 @@ private module ImplCommon { exists(Node mid | parameterValueFlowCand(p, mid) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) or // flow through a callable exists(Node arg | parameterValueFlowCand(p, arg) and argumentValueFlowsThroughCand(arg, node) and - compatibleTypes(p.getType(), node.getType()) + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) ) } @@ -443,7 +443,7 @@ private module ImplCommon { argumentValueFlowsThroughCand0(call, arg, kind) | out = getAnOutNode(call, kind) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } @@ -531,7 +531,7 @@ private module ImplCommon { exists(Node mid | parameterValueFlow(p, mid, cc) and step(mid, node) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) or @@ -539,7 +539,7 @@ private module ImplCommon { exists(Node arg | parameterValueFlow(p, arg, cc) and argumentValueFlowsThrough(arg, node, cc) and - compatibleTypes(p.getType(), node.getType()) and + compatibleTypes(getErasedNodeType(p), getErasedNodeType(node)) and not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) ) } @@ -574,7 +574,7 @@ private module ImplCommon { | out = getAnOutNode(call, kind) and not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and - compatibleTypes(arg.getType(), out.getType()) + compatibleTypes(getErasedNodeType(arg), getErasedNodeType(out)) ) } } @@ -860,4 +860,10 @@ private module ImplCommon { or result = viableCallable(call) and cc instanceof CallContextReturn } + + pragma[noinline] + DataFlowType getErasedNodeType(Node n) { result = getErasedRepr(n.getType()) } + + pragma[noinline] + DataFlowType getErasedNodeTypeBound(Node n) { result = getErasedRepr(n.getTypeBound()) } } diff --git a/java/ql/test/query-tests/Nullness/B.java b/java/ql/test/query-tests/Nullness/B.java index c0d9171041b..193d61e497f 100644 --- a/java/ql/test/query-tests/Nullness/B.java +++ b/java/ql/test/query-tests/Nullness/B.java @@ -324,4 +324,51 @@ public class B { if (x != null) y.hashCode(); // OK if (y != null) x.hashCode(); // OK } + + public void corrConds3(Object y) { + Object x = null; + if(y instanceof String) { + x = new Object(); + } + if(y instanceof String) { + x.hashCode(); // OK + } + } + + public void corrConds4(Object y) { + Object x = null; + if(!(y instanceof String)) { + x = new Object(); + } + if(!(y instanceof String)) { + x.hashCode(); // OK + } + } + + public void corrConds5(Object y, Object z) { + Object x = null; + if(y == z) { + x = new Object(); + } + if(y == z) { + x.hashCode(); // OK + } + + Object x2 = null; + if(y != z) { + x2 = new Object(); + } + if(y != z) { + x2.hashCode(); // OK + } + + Object x3 = null; + if(y != z) { + x3 = new Object(); + } + if(!(y == z)) { + x3.hashCode(); // OK + } + } + } diff --git a/javascript/ql/src/Statements/UseOfReturnlessFunction.ql b/javascript/ql/src/Statements/UseOfReturnlessFunction.ql index d782bf7e7e8..35825b01bb4 100644 --- a/javascript/ql/src/Statements/UseOfReturnlessFunction.ql +++ b/javascript/ql/src/Statements/UseOfReturnlessFunction.ql @@ -222,6 +222,7 @@ where not lastStatementHasNoEffect(func) and // anonymous one-shot closure. Those are used in weird ways and we ignore them. not oneshotClosure(call) and - not hasNonVoidReturnType(func) + not hasNonVoidReturnType(func) and + not call.getEnclosingExpr() instanceof SuperCall select call, msg, func, name diff --git a/javascript/ql/src/javascript.qll b/javascript/ql/src/javascript.qll index 9a183afffe5..08b305c159b 100644 --- a/javascript/ql/src/javascript.qll +++ b/javascript/ql/src/javascript.qll @@ -76,6 +76,7 @@ import semmle.javascript.frameworks.Electron import semmle.javascript.frameworks.Files import semmle.javascript.frameworks.Firebase import semmle.javascript.frameworks.jQuery +import semmle.javascript.frameworks.Handlebars import semmle.javascript.frameworks.LodashUnderscore import semmle.javascript.frameworks.Logging import semmle.javascript.frameworks.HttpFrameworks diff --git a/javascript/ql/src/semmle/javascript/frameworks/Handlebars.qll b/javascript/ql/src/semmle/javascript/frameworks/Handlebars.qll new file mode 100644 index 00000000000..6879d4b7699 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/frameworks/Handlebars.qll @@ -0,0 +1,29 @@ +/** + * Provides classes for working with Handlebars code. + */ + +import javascript + +module Handlebars { + /** + * A reference to the Handlebars library. + */ + class Handlebars extends DataFlow::SourceNode { + Handlebars() { + this.accessesGlobal("handlebars") + or + this.accessesGlobal("Handlebars") + or + this = DataFlow::moduleImport("handlebars") + or + this.hasUnderlyingType("Handlebars") + } + } + + /** + * A new instantiation of a Handlebars.SafeString. + */ + class SafeString extends DataFlow::NewNode { + SafeString() { this = any(Handlebars h).getAConstructorInvocation("SafeString") } + } +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll index 52c4a9b9332..76c4a29f971 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll @@ -95,6 +95,8 @@ module DomBasedXss { mcn.getMethodName() = m and this = mcn.getArgument(1) ) + or + this = any(Handlebars::SafeString s).getAnArgument() } } diff --git a/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected index f29d3e5de0f..2e986a06dea 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected @@ -315,6 +315,9 @@ nodes | tst.js:285:59:285:65 | tainted | | tst.js:285:59:285:65 | tainted | | tst.js:285:59:285:65 | tainted | +| tst.js:297:35:297:42 | location | +| tst.js:297:35:297:42 | location | +| tst.js:297:35:297:42 | location | | v-html.vue:2:8:2:23 | v-html=tainted | | v-html.vue:2:8:2:23 | v-html=tainted | | v-html.vue:6:42:6:58 | document.location | @@ -604,6 +607,7 @@ edges | tst.js:282:19:282:29 | window.name | tst.js:282:9:282:29 | tainted | | tst.js:282:19:282:29 | window.name | tst.js:282:9:282:29 | tainted | | tst.js:285:59:285:65 | tainted | tst.js:285:59:285:65 | tainted | +| tst.js:297:35:297:42 | location | tst.js:297:35:297:42 | location | | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | @@ -693,6 +697,7 @@ edges | tst.js:285:59:285:65 | tainted | tst.js:282:9:282:29 | tainted | tst.js:285:59:285:65 | tainted | Cross-site scripting vulnerability due to $@. | tst.js:282:9:282:29 | tainted | user-provided value | | tst.js:285:59:285:65 | tainted | tst.js:282:19:282:29 | window.name | tst.js:285:59:285:65 | tainted | Cross-site scripting vulnerability due to $@. | tst.js:282:19:282:29 | window.name | user-provided value | | tst.js:285:59:285:65 | tainted | tst.js:285:59:285:65 | tainted | tst.js:285:59:285:65 | tainted | Cross-site scripting vulnerability due to $@. | tst.js:285:59:285:65 | tainted | user-provided value | +| tst.js:297:35:297:42 | location | tst.js:297:35:297:42 | location | tst.js:297:35:297:42 | location | Cross-site scripting vulnerability due to $@. | tst.js:297:35:297:42 | location | user-provided value | | v-html.vue:2:8:2:23 | v-html=tainted | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | Cross-site scripting vulnerability due to $@. | v-html.vue:6:42:6:58 | document.location | user-provided value | | winjs.js:3:43:3:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:3:43:3:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value | | winjs.js:4:43:4:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:4:43:4:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/tst.js b/javascript/ql/test/query-tests/Security/CWE-079/tst.js index 53840d61240..780d56e90e1 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/tst.js @@ -292,3 +292,7 @@ function flowThroughPropertyNames() { for (var p in obj) $(p); // OK } + +function handlebarsSafeString() { + return new Handlebars.SafeString(location); // NOT OK! +} diff --git a/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.js b/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.js index 5c6634cde7b..dbedb3dee3c 100644 --- a/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.js +++ b/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.js @@ -95,3 +95,15 @@ +function() { console.log("FOO"); }.call(this); + +class Foo { + constructor() { + console.log("FOO"); + } +} + +class Bar extends Foo { + constructor() { + console.log(super()); // OK. + } +}