From de7c9bdd9b49dfe3d2b37f69248c2a196a8ba280 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:07:30 +0200 Subject: [PATCH 1/7] Data flow: Add consistency checks to shared ql pack --- .../internal/DataFlowImplConsistency.qll | 290 ++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll new file mode 100644 index 00000000000..a24f8ab794a --- /dev/null +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll @@ -0,0 +1,290 @@ +/** + * Provides consistency queries for checking invariants in the language-specific + * data-flow classes and predicates. + */ + +private import codeql.dataflow.DataFlow as DF +private import codeql.dataflow.TaintTracking as TT + +signature module InputSig { + /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ + default predicate uniqueEnclosingCallableExclude(DataFlowLang::Node n) { none() } + + /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ + default predicate uniqueCallEnclosingCallableExclude(DataFlowLang::DataFlowCall call) { none() } + + /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ + default predicate uniqueNodeLocationExclude(DataFlowLang::Node n) { none() } + + /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ + default predicate missingLocationExclude(DataFlowLang::Node n) { none() } + + /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ + default predicate postWithInFlowExclude(DataFlowLang::Node n) { none() } + + /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ + default predicate argHasPostUpdateExclude(DataFlowLang::ArgumentNode n) { none() } + + /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ + default predicate reverseReadExclude(DataFlowLang::Node n) { none() } + + /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ + default predicate postHasUniquePreExclude(DataFlowLang::PostUpdateNode n) { none() } + + /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ + default predicate uniquePostUpdateExclude(DataFlowLang::Node n) { none() } + + /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ + default predicate viableImplInCallContextTooLargeExclude( + DataFlowLang::DataFlowCall call, DataFlowLang::DataFlowCall ctx, + DataFlowLang::DataFlowCallable callable + ) { + none() + } + + /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ + default predicate uniqueParameterNodeAtPositionExclude( + DataFlowLang::DataFlowCallable c, DataFlowLang::ParameterPosition pos, DataFlowLang::Node p + ) { + none() + } + + /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ + default predicate uniqueParameterNodePositionExclude( + DataFlowLang::DataFlowCallable c, DataFlowLang::ParameterPosition pos, DataFlowLang::Node p + ) { + none() + } + + /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ + default predicate identityLocalStepExclude(DataFlowLang::Node n) { none() } +} + +module MakeConsistency< + DF::InputSig DataFlowLang, TT::InputSig TaintTrackingLang, + InputSig Input> +{ + private import DataFlowLang + private import TaintTrackingLang + + final private class NodeFinal = Node; + + private class RelevantNode extends NodeFinal { + RelevantNode() { + this instanceof ArgumentNode or + this instanceof ParameterNode or + this instanceof ReturnNode or + this = getAnOutNode(_, _) or + simpleLocalFlowStep(this, _) or + simpleLocalFlowStep(_, this) or + jumpStep(this, _) or + jumpStep(_, this) or + storeStep(this, _, _) or + storeStep(_, _, this) or + readStep(this, _, _) or + readStep(_, _, this) or + defaultAdditionalTaintStep(this, _) or + defaultAdditionalTaintStep(_, this) + } + } + + query predicate uniqueEnclosingCallable(Node n, string msg) { + exists(int c | + n instanceof RelevantNode and + c = count(nodeGetEnclosingCallable(n)) and + c != 1 and + not Input::uniqueEnclosingCallableExclude(n) and + msg = "Node should have one enclosing callable but has " + c + "." + ) + } + + query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { + exists(int c | + c = count(call.getEnclosingCallable()) and + c != 1 and + not Input::uniqueCallEnclosingCallableExclude(call) and + msg = "Call should have one enclosing callable but has " + c + "." + ) + } + + query predicate uniqueType(Node n, string msg) { + exists(int c | + n instanceof RelevantNode and + c = count(getNodeType(n)) and + c != 1 and + msg = "Node should have one type but has " + c + "." + ) + } + + query predicate uniqueNodeLocation(Node n, string msg) { + exists(int c | + c = + count(string filepath, int startline, int startcolumn, int endline, int endcolumn | + n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + ) and + c != 1 and + not Input::uniqueNodeLocationExclude(n) and + msg = "Node should have one location but has " + c + "." + ) + } + + query predicate missingLocation(string msg) { + exists(int c | + c = + strictcount(Node n | + not n.hasLocationInfo(_, _, _, _, _) and + not Input::missingLocationExclude(n) + ) and + msg = "Nodes without location: " + c + ) + } + + query predicate uniqueNodeToString(Node n, string msg) { + exists(int c | + c = count(n.toString()) and + c != 1 and + msg = "Node should have one toString but has " + c + "." + ) + } + + query predicate missingToString(string msg) { + exists(int c | + c = strictcount(Node n | not exists(n.toString())) and + msg = "Nodes without toString: " + c + ) + } + + query predicate parameterCallable(ParameterNode p, string msg) { + exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and + msg = "Callable mismatch for parameter." + } + + query predicate localFlowIsLocal(Node n1, Node n2, string msg) { + simpleLocalFlowStep(n1, n2) and + nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and + msg = "Local flow step does not preserve enclosing callable." + } + + query predicate readStepIsLocal(Node n1, Node n2, string msg) { + readStep(n1, _, n2) and + nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and + msg = "Read step does not preserve enclosing callable." + } + + query predicate storeStepIsLocal(Node n1, Node n2, string msg) { + storeStep(n1, _, n2) and + nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and + msg = "Store step does not preserve enclosing callable." + } + + private DataFlowType typeRepr() { result = getNodeType(_) } + + query predicate compatibleTypesReflexive(DataFlowType t, string msg) { + t = typeRepr() and + not compatibleTypes(t, t) and + msg = "Type compatibility predicate is not reflexive." + } + + query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { + isUnreachableInCall(n, call) and + exists(DataFlowCallable c | + c = nodeGetEnclosingCallable(n) and + not viableCallable(call) = c + ) and + msg = "Call context for isUnreachableInCall is inconsistent with call graph." + } + + query predicate localCallNodes(DataFlowCall call, Node n, string msg) { + ( + n = getAnOutNode(call, _) and + msg = "OutNode and call does not share enclosing callable." + or + isArgumentNode(n, call, _) and + msg = "ArgumentNode and call does not share enclosing callable." + ) and + nodeGetEnclosingCallable(n) != call.getEnclosingCallable() + } + + query predicate postIsNotPre(PostUpdateNode n, string msg) { + n = n.getPreUpdateNode() and + msg = "PostUpdateNode should not equal its pre-update node." + } + + query predicate postHasUniquePre(PostUpdateNode n, string msg) { + not Input::postHasUniquePreExclude(n) and + exists(int c | + c = count(n.getPreUpdateNode()) and + c != 1 and + msg = "PostUpdateNode should have one pre-update node but has " + c + "." + ) + } + + query predicate uniquePostUpdate(Node n, string msg) { + not Input::uniquePostUpdateExclude(n) and + 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and + msg = "Node has multiple PostUpdateNodes." + } + + query predicate postIsInSameCallable(PostUpdateNode n, string msg) { + nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and + msg = "PostUpdateNode does not share callable with its pre-update node." + } + + private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } + + query predicate reverseRead(Node n, string msg) { + exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and + not Input::reverseReadExclude(n) and + msg = "Origin of readStep is missing a PostUpdateNode." + } + + query predicate argHasPostUpdate(ArgumentNode n, string msg) { + not hasPost(n) and + not Input::argHasPostUpdateExclude(n) and + msg = "ArgumentNode is missing PostUpdateNode." + } + + query predicate postWithInFlow(PostUpdateNode n, string msg) { + not clearsContent(n, _) and + simpleLocalFlowStep(_, n) and + not Input::postWithInFlowExclude(n) and + msg = "PostUpdateNode should not be the target of local flow." + } + + query predicate viableImplInCallContextTooLarge( + DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable + ) { + callable = viableImplInCallContext(call, ctx) and + not callable = viableCallable(call) and + not Input::viableImplInCallContextTooLargeExclude(call, ctx, callable) + } + + query predicate uniqueParameterNodeAtPosition( + DataFlowCallable c, ParameterPosition pos, Node p, string msg + ) { + not Input::uniqueParameterNodeAtPositionExclude(c, pos, p) and + isParameterNode(p, c, pos) and + not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and + msg = "Parameters with overlapping positions." + } + + query predicate uniqueParameterNodePosition( + DataFlowCallable c, ParameterPosition pos, Node p, string msg + ) { + not Input::uniqueParameterNodePositionExclude(c, pos, p) and + isParameterNode(p, c, pos) and + not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and + msg = "Parameter node with multiple positions." + } + + query predicate uniqueContentApprox(Content c, string msg) { + not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and + msg = "Non-unique content approximation." + } + + query predicate identityLocalStep(Node n, string msg) { + simpleLocalFlowStep(n, n) and + not Input::identityLocalStepExclude(n) and + msg = "Node steps to itself" + } +} From c4b626a416bc188e57948ebd5a14683b273e3628 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:08:26 +0200 Subject: [PATCH 2/7] Ruby: Use data flow consistency checks from shared pack --- config/identical-files.json | 3 +- .../DataFlowConsistency.ql | 27 +- .../internal/DataFlowImplConsistency.qll | 299 ------------------ 3 files changed, 17 insertions(+), 312 deletions(-) delete mode 100644 ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll diff --git a/config/identical-files.json b/config/identical-files.json index 214fc402c4f..9117899c2c0 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -61,7 +61,6 @@ "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll", "csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll", "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll", - "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll", "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll" ], "DataFlow Java/C#/Go/Ruby/Python/Swift Flow Summaries": [ @@ -561,4 +560,4 @@ "python/ql/test/experimental/dataflow/model-summaries/InlineTaintTest.ext.yml", "python/ql/test/experimental/dataflow/model-summaries/NormalDataflowTest.ext.yml" ] -} +} \ No newline at end of file diff --git a/ruby/ql/consistency-queries/DataFlowConsistency.ql b/ruby/ql/consistency-queries/DataFlowConsistency.ql index 46384305f7a..6cf0fe9a282 100644 --- a/ruby/ql/consistency-queries/DataFlowConsistency.ql +++ b/ruby/ql/consistency-queries/DataFlowConsistency.ql @@ -1,13 +1,16 @@ -import codeql.ruby.AST -import codeql.ruby.CFG -import codeql.ruby.DataFlow::DataFlow -import codeql.ruby.dataflow.internal.DataFlowPrivate -import codeql.ruby.dataflow.internal.DataFlowImplConsistency::Consistency +import codeql.ruby.DataFlow::DataFlow as DataFlow +private import codeql.ruby.AST +private import codeql.ruby.CFG +private import codeql.ruby.dataflow.internal.DataFlowImplSpecific +private import codeql.ruby.dataflow.internal.TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -private class MyConsistencyConfiguration extends ConsistencyConfiguration { - override predicate postWithInFlowExclude(Node n) { n instanceof FlowSummaryNode } +private module Input implements InputSig { + private import RubyDataFlow - override predicate argHasPostUpdateExclude(ArgumentNode n) { + predicate postWithInFlowExclude(Node n) { n instanceof FlowSummaryNode } + + predicate argHasPostUpdateExclude(ArgumentNode n) { n instanceof BlockArgumentNode or n instanceof FlowSummaryNode @@ -17,7 +20,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { not isNonConstantExpr(getAPostUpdateNodeForArg(n.asExpr())) } - override predicate postHasUniquePreExclude(PostUpdateNode n) { + predicate postHasUniquePreExclude(PostUpdateNode n) { exists(CfgNodes::ExprCfgNode e, CfgNodes::ExprCfgNode arg | e = getAPostUpdateNodeForArg(arg) and e != arg and @@ -25,7 +28,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { ) } - override predicate uniquePostUpdateExclude(Node n) { + predicate uniquePostUpdateExclude(Node n) { exists(CfgNodes::ExprCfgNode e, CfgNodes::ExprCfgNode arg | e = getAPostUpdateNodeForArg(arg) and e != arg and @@ -34,7 +37,9 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { } } -query predicate multipleToString(Node n, string s) { +import MakeConsistency + +query predicate multipleToString(DataFlow::Node n, string s) { s = strictconcat(n.toString(), ",") and strictcount(n.toString()) > 1 } diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll deleted file mode 100644 index e154491f795..00000000000 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll +++ /dev/null @@ -1,299 +0,0 @@ -/** - * Provides consistency queries for checking invariants in the language-specific - * data-flow classes and predicates. - */ - -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public - -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() - - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() - or - none() - } - - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." - } - - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." - ) - } - - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." - } - - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" - } -} From 5c8367a6956fa0aead0d88c7f769ed529b769e14 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:14:43 +0200 Subject: [PATCH 3/7] C#: Use data flow consistency checks from shared pack --- config/identical-files.json | 1 - .../DataFlowConsistency.ql | 37 +-- .../internal/DataFlowImplConsistency.qll | 299 ------------------ 3 files changed, 19 insertions(+), 318 deletions(-) delete mode 100644 csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll diff --git a/config/identical-files.json b/config/identical-files.json index 9117899c2c0..a2074276065 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -59,7 +59,6 @@ "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll", "cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll", "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll", - "csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll", "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll", "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll" ], diff --git a/csharp/ql/consistency-queries/DataFlowConsistency.ql b/csharp/ql/consistency-queries/DataFlowConsistency.ql index 4ac709fd366..3fcda210961 100644 --- a/csharp/ql/consistency-queries/DataFlowConsistency.ql +++ b/csharp/ql/consistency-queries/DataFlowConsistency.ql @@ -1,12 +1,13 @@ import csharp import cil -import semmle.code.csharp.dataflow.internal.DataFlowPrivate -import semmle.code.csharp.dataflow.internal.DataFlowPublic -import semmle.code.csharp.dataflow.internal.DataFlowDispatch -import semmle.code.csharp.dataflow.internal.DataFlowImplConsistency::Consistency +private import semmle.code.csharp.dataflow.internal.DataFlowImplSpecific +private import semmle.code.csharp.dataflow.internal.TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -private class MyConsistencyConfiguration extends ConsistencyConfiguration { - override predicate uniqueEnclosingCallableExclude(Node n) { +private module Input implements InputSig { + private import CsharpDataFlow + + predicate uniqueEnclosingCallableExclude(Node n) { // TODO: Remove once static initializers are folded into the // static constructors exists(ControlFlow::Node cfn | @@ -15,7 +16,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { ) } - override predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { + predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { // TODO: Remove once static initializers are folded into the // static constructors exists(ControlFlow::Node cfn | @@ -24,25 +25,25 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { ) } - override predicate uniqueNodeLocationExclude(Node n) { + predicate uniqueNodeLocationExclude(Node n) { // Methods with multiple implementations n instanceof ParameterNode or - this.missingLocationExclude(n) + missingLocationExclude(n) } - override predicate missingLocationExclude(Node n) { + predicate missingLocationExclude(Node n) { // Some CIL methods are missing locations n.asParameter() instanceof CIL::Parameter } - override predicate postWithInFlowExclude(Node n) { + predicate postWithInFlowExclude(Node n) { n instanceof FlowSummaryNode or n.asExpr().(ObjectCreation).hasInitializer() } - override predicate argHasPostUpdateExclude(ArgumentNode n) { + predicate argHasPostUpdateExclude(ArgumentNode n) { n instanceof FlowSummaryNode or not exists(LocalFlow::getAPostUpdateNodeForArg(n.getControlFlowNode())) @@ -54,7 +55,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { n.asExpr() instanceof CIL::Expr } - override predicate postHasUniquePreExclude(PostUpdateNode n) { + predicate postHasUniquePreExclude(PostUpdateNode n) { exists(ControlFlow::Nodes::ExprNode e, ControlFlow::Nodes::ExprNode arg | e = LocalFlow::getAPostUpdateNodeForArg(arg) and e != arg and @@ -62,7 +63,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { ) } - override predicate uniquePostUpdateExclude(Node n) { + predicate uniquePostUpdateExclude(Node n) { exists(ControlFlow::Nodes::ExprNode e, ControlFlow::Nodes::ExprNode arg | e = LocalFlow::getAPostUpdateNodeForArg(arg) and e != arg and @@ -70,12 +71,12 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { ) } - override predicate reverseReadExclude(Node n) { n.asExpr() = any(AwaitExpr ae).getExpr() } - - override predicate identityLocalStepExclude(Node n) { none() } + predicate reverseReadExclude(Node n) { n.asExpr() = any(AwaitExpr ae).getExpr() } } -query predicate multipleToString(Node n, string s) { +import MakeConsistency + +query predicate multipleToString(DataFlow::Node n, string s) { s = strictconcat(n.toString(), ",") and strictcount(n.toString()) > 1 } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll deleted file mode 100644 index e154491f795..00000000000 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll +++ /dev/null @@ -1,299 +0,0 @@ -/** - * Provides consistency queries for checking invariants in the language-specific - * data-flow classes and predicates. - */ - -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public - -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() - - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() - or - none() - } - - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." - } - - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." - ) - } - - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." - } - - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" - } -} From fefe64bf0cafb9652398dc50797a0ad3afa7b330 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:20:44 +0200 Subject: [PATCH 4/7] Java: Use data flow consistency checks from shared pack --- config/identical-files.json | 1 - .../internal/DataFlowImplConsistency.qll | 300 +----------------- .../dataflow/internal/DataFlowPrivate.qll | 7 - 3 files changed, 9 insertions(+), 299 deletions(-) diff --git a/config/identical-files.json b/config/identical-files.json index a2074276065..5785bf71c70 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -56,7 +56,6 @@ "swift/ql/lib/codeql/swift/dataflow/internal/tainttracking1/TaintTrackingImpl.qll" ], "DataFlow Java/C++/C#/Python/Ruby/Swift Consistency checks": [ - "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll", "cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll", "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll", "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll", diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll index e154491f795..1dfa24fffac 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll @@ -3,297 +3,15 @@ * data-flow classes and predicates. */ -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public +private import java +private import DataFlowImplSpecific +private import TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() - - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() - or - none() - } - - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." - } - - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." - ) - } - - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." - } - - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" +private module Input implements InputSig { + predicate argHasPostUpdateExclude(JavaDataFlow::ArgumentNode n) { + n.getType() instanceof ImmutableType or n instanceof Public::ImplicitVarargsArray } } + +module Consistency = MakeConsistency; diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index 212232e077a..8b70c43fed9 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -8,7 +8,6 @@ private import ContainerFlow private import semmle.code.java.dataflow.FlowSteps private import semmle.code.java.dataflow.FlowSummary private import FlowSummaryImpl as FlowSummaryImpl -private import DataFlowImplConsistency private import DataFlowNodes private import codeql.dataflow.VariableCapture as VariableCapture import DataFlowNodes::Private @@ -572,12 +571,6 @@ ContentApprox getContentApprox(Content c) { c instanceof SyntheticFieldContent and result = TSyntheticFieldApproxContent() } -private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration { - override predicate argHasPostUpdateExclude(ArgumentNode n) { - n.getType() instanceof ImmutableType or n instanceof ImplicitVarargsArray - } -} - /** * Holds if the the content `c` is a container. */ From db304d118b779e975aff1acfc5fee29b0baed72b Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:24:50 +0200 Subject: [PATCH 5/7] C++: Use data flow consistency checks from shared pack --- config/identical-files.json | 2 - .../internal/DataFlowImplConsistency.qll | 306 +----------------- .../cpp/dataflow/internal/DataFlowPrivate.qll | 17 - .../internal/DataFlowImplConsistency.qll | 302 +---------------- .../ir/dataflow/internal/DataFlowPrivate.qll | 9 - 5 files changed, 28 insertions(+), 608 deletions(-) diff --git a/config/identical-files.json b/config/identical-files.json index 5785bf71c70..5607a25cbc9 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -56,8 +56,6 @@ "swift/ql/lib/codeql/swift/dataflow/internal/tainttracking1/TaintTrackingImpl.qll" ], "DataFlow Java/C++/C#/Python/Ruby/Swift Consistency checks": [ - "cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll", - "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll", "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll", "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll" ], diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll index e154491f795..229031e0149 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll @@ -3,297 +3,25 @@ * data-flow classes and predicates. */ -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public +private import cpp +private import DataFlowImplSpecific +private import TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() - - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() +private module Input implements InputSig { + predicate argHasPostUpdateExclude(Private::ArgumentNode n) { + // Is the null pointer (or something that's not really a pointer) + exists(n.asExpr().getValue()) or - none() - } - - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." - } - - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." + // Isn't a pointer or is a pointer to const + forall(DerivedType dt | dt = n.asExpr().getActualType() | + dt.getBaseType().isConst() + or + dt.getBaseType() instanceof RoutineType ) - } - - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." - } - - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" + // The above list of cases isn't exhaustive, but it narrows down the + // consistency alerts enough that most of them are interesting. } } + +module Consistency = MakeConsistency; diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll index a6f00f30b27..00eca92b3e4 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll @@ -2,7 +2,6 @@ private import cpp private import DataFlowUtil private import DataFlowDispatch private import FlowVar -private import DataFlowImplConsistency private import codeql.util.Unit /** Gets the callable in which this node occurs. */ @@ -297,22 +296,6 @@ class ContentApprox = Unit; pragma[inline] ContentApprox getContentApprox(Content c) { any() } -private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration { - override predicate argHasPostUpdateExclude(ArgumentNode n) { - // Is the null pointer (or something that's not really a pointer) - exists(n.asExpr().getValue()) - or - // Isn't a pointer or is a pointer to const - forall(DerivedType dt | dt = n.asExpr().getActualType() | - dt.getBaseType().isConst() - or - dt.getBaseType() instanceof RoutineType - ) - // The above list of cases isn't exhaustive, but it narrows down the - // consistency alerts enough that most of them are interesting. - } -} - /** * Gets an additional term that is added to the `join` and `branch` computations to reflect * an additional forward or backwards branching factor that is not taken into account diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll index e154491f795..c32f63a619d 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll @@ -3,297 +3,17 @@ * data-flow classes and predicates. */ -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public +private import cpp +private import DataFlowImplSpecific +private import TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() - - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() - or - none() - } - - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." - } - - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." - ) - } - - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." - } - - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" +private module Input implements InputSig { + predicate argHasPostUpdateExclude(Private::ArgumentNode n) { + // The rules for whether an IR argument gets a post-update node are too + // complex to model here. + any() } } + +module Consistency = MakeConsistency; diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 329164e0fd0..b5515fbe5e3 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -2,7 +2,6 @@ private import cpp as Cpp private import DataFlowUtil private import semmle.code.cpp.ir.IR private import DataFlowDispatch -private import DataFlowImplConsistency private import semmle.code.cpp.ir.internal.IRCppLanguage private import SsaInternals as Ssa private import DataFlowImplCommon as DataFlowImplCommon @@ -1011,14 +1010,6 @@ ContentApprox getContentApprox(Content c) { ) } -private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration { - override predicate argHasPostUpdateExclude(ArgumentNode n) { - // The rules for whether an IR argument gets a post-update node are too - // complex to model here. - any() - } -} - /** * A local flow relation that includes both local steps, read steps and * argument-to-return flow through summarized functions. From 9af706c2a58867483593a94389afeb79b82514dc Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:27:47 +0200 Subject: [PATCH 6/7] Swift: Use data flow consistency checks from shared pack --- config/identical-files.json | 3 +- .../internal/DataFlowImplConsistency.qll | 298 +----------------- 2 files changed, 7 insertions(+), 294 deletions(-) diff --git a/config/identical-files.json b/config/identical-files.json index 5607a25cbc9..f68806818cf 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -56,8 +56,7 @@ "swift/ql/lib/codeql/swift/dataflow/internal/tainttracking1/TaintTrackingImpl.qll" ], "DataFlow Java/C++/C#/Python/Ruby/Swift Consistency checks": [ - "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll", - "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll" + "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll" ], "DataFlow Java/C#/Go/Ruby/Python/Swift Flow Summaries": [ "java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll", diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll index e154491f795..e9de11852a6 100644 --- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll +++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll @@ -3,297 +3,11 @@ * data-flow classes and predicates. */ -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public +private import swift +private import DataFlowImplSpecific +private import TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() +private module Input implements InputSig { } - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() - or - none() - } - - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." - } - - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." - ) - } - - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." - } - - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" - } -} +module Consistency = MakeConsistency; From 253f932d2a1d52d815102f597e4ccc93218ce61a Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 30 Aug 2023 14:31:08 +0200 Subject: [PATCH 7/7] Python: Use data flow consistency checks from shared pack --- config/identical-files.json | 3 - .../new/internal/DataFlowImplConsistency.qll | 313 ++---------------- .../dataflow/TestUtil/DataFlowConsistency.qll | 44 +-- 3 files changed, 35 insertions(+), 325 deletions(-) diff --git a/config/identical-files.json b/config/identical-files.json index f68806818cf..f814b0f1f81 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -55,9 +55,6 @@ "ruby/ql/lib/codeql/ruby/dataflow/internal/tainttracking1/TaintTrackingImpl.qll", "swift/ql/lib/codeql/swift/dataflow/internal/tainttracking1/TaintTrackingImpl.qll" ], - "DataFlow Java/C++/C#/Python/Ruby/Swift Consistency checks": [ - "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll" - ], "DataFlow Java/C#/Go/Ruby/Python/Swift Flow Summaries": [ "java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll", "csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll", diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll index e154491f795..2513a4aa3bd 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll @@ -3,297 +3,50 @@ * data-flow classes and predicates. */ -private import DataFlowImplSpecific::Private -private import DataFlowImplSpecific::Public -private import tainttracking1.TaintTrackingParameter::Private -private import tainttracking1.TaintTrackingParameter::Public +private import python +private import DataFlowImplSpecific +private import TaintTrackingImplSpecific +private import codeql.dataflow.internal.DataFlowImplConsistency -module Consistency { - private newtype TConsistencyConfiguration = MkConsistencyConfiguration() +private module Input implements InputSig { + private import Private + private import Public - /** A class for configuring the consistency queries. */ - class ConsistencyConfiguration extends TConsistencyConfiguration { - string toString() { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */ - predicate uniqueEnclosingCallableExclude(Node n) { none() } - - /** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */ - predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */ - predicate uniqueNodeLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `missingLocation`. */ - predicate missingLocationExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */ - predicate postWithInFlowExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */ - predicate argHasPostUpdateExclude(ArgumentNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `reverseRead`. */ - predicate reverseReadExclude(Node n) { none() } - - /** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */ - predicate postHasUniquePreExclude(PostUpdateNode n) { none() } - - /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */ - predicate uniquePostUpdateExclude(Node n) { none() } - - /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */ - predicate viableImplInCallContextTooLargeExclude( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */ - predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */ - predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { - none() - } - - /** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */ - predicate identityLocalStepExclude(Node n) { none() } - } - - private class RelevantNode extends Node { - RelevantNode() { - this instanceof ArgumentNode or - this instanceof ParameterNode or - this instanceof ReturnNode or - this = getAnOutNode(_, _) or - simpleLocalFlowStep(this, _) or - simpleLocalFlowStep(_, this) or - jumpStep(this, _) or - jumpStep(_, this) or - storeStep(this, _, _) or - storeStep(_, _, this) or - readStep(this, _, _) or - readStep(_, _, this) or - defaultAdditionalTaintStep(this, _) or - defaultAdditionalTaintStep(_, this) - } - } - - query predicate uniqueEnclosingCallable(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(nodeGetEnclosingCallable(n)) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and - msg = "Node should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) { - exists(int c | - c = count(call.getEnclosingCallable()) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and - msg = "Call should have one enclosing callable but has " + c + "." - ) - } - - query predicate uniqueType(Node n, string msg) { - exists(int c | - n instanceof RelevantNode and - c = count(getNodeType(n)) and - c != 1 and - msg = "Node should have one type but has " + c + "." - ) - } - - query predicate uniqueNodeLocation(Node n, string msg) { - exists(int c | - c = - count(string filepath, int startline, int startcolumn, int endline, int endcolumn | - n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - ) and - c != 1 and - not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and - msg = "Node should have one location but has " + c + "." - ) - } - - query predicate missingLocation(string msg) { - exists(int c | - c = - strictcount(Node n | - not n.hasLocationInfo(_, _, _, _, _) and - not any(ConsistencyConfiguration conf).missingLocationExclude(n) - ) and - msg = "Nodes without location: " + c - ) - } - - query predicate uniqueNodeToString(Node n, string msg) { - exists(int c | - c = count(n.toString()) and - c != 1 and - msg = "Node should have one toString but has " + c + "." - ) - } - - query predicate missingToString(string msg) { - exists(int c | - c = strictcount(Node n | not exists(n.toString())) and - msg = "Nodes without toString: " + c - ) - } - - query predicate parameterCallable(ParameterNode p, string msg) { - exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and - msg = "Callable mismatch for parameter." - } - - query predicate localFlowIsLocal(Node n1, Node n2, string msg) { - simpleLocalFlowStep(n1, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Local flow step does not preserve enclosing callable." - } - - query predicate readStepIsLocal(Node n1, Node n2, string msg) { - readStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Read step does not preserve enclosing callable." - } - - query predicate storeStepIsLocal(Node n1, Node n2, string msg) { - storeStep(n1, _, n2) and - nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and - msg = "Store step does not preserve enclosing callable." - } - - private DataFlowType typeRepr() { result = getNodeType(_) } - - query predicate compatibleTypesReflexive(DataFlowType t, string msg) { - t = typeRepr() and - not compatibleTypes(t, t) and - msg = "Type compatibility predicate is not reflexive." - } - - query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) { - isUnreachableInCall(n, call) and - exists(DataFlowCallable c | - c = nodeGetEnclosingCallable(n) and - not viableCallable(call) = c - ) and - msg = "Call context for isUnreachableInCall is inconsistent with call graph." - } - - query predicate localCallNodes(DataFlowCall call, Node n, string msg) { - ( - n = getAnOutNode(call, _) and - msg = "OutNode and call does not share enclosing callable." - or - n.(ArgumentNode).argumentOf(call, _) and - msg = "ArgumentNode and call does not share enclosing callable." - ) and - nodeGetEnclosingCallable(n) != call.getEnclosingCallable() - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a result of `getPreUpdateNode` to be an - // instance of `PostUpdateNode`. - private Node getPre(PostUpdateNode n) { - result = n.getPreUpdateNode() + predicate argHasPostUpdateExclude(ArgumentNode n) { + exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isStarArgs(_)) or - none() + exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isDictSplat()) } - query predicate postIsNotPre(PostUpdateNode n, string msg) { - getPre(n) = n and - msg = "PostUpdateNode should not equal its pre-update node." + predicate reverseReadExclude(Node n) { + // since `self`/`cls` parameters can be marked as implicit argument to `super()`, + // they will have PostUpdateNodes. We have a read-step from the synthetic `**kwargs` + // parameter, but dataflow-consistency queries should _not_ complain about there not + // being a post-update node for the synthetic `**kwargs` parameter. + n instanceof SynthDictSplatParameterNode } - query predicate postHasUniquePre(PostUpdateNode n, string msg) { - not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and - exists(int c | - c = count(n.getPreUpdateNode()) and - c != 1 and - msg = "PostUpdateNode should have one pre-update node but has " + c + "." + predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) { + // For normal parameters that can both be passed as positional arguments or keyword + // arguments, we currently have parameter positions for both cases.. + // + // TODO: Figure out how bad breaking this consistency check is + exists(Function func, Parameter param | + c.getScope() = func and + p = parameterNode(param) and + c.getParameter(pos) = p and + param = func.getArg(_) and + param = func.getArgByName(_) ) } - query predicate uniquePostUpdate(Node n, string msg) { - not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and - 1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and - msg = "Node has multiple PostUpdateNodes." + predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { + not exists(call.getLocation().getFile().getRelativePath()) } - query predicate postIsInSameCallable(PostUpdateNode n, string msg) { - nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and - msg = "PostUpdateNode does not share callable with its pre-update node." - } - - private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) } - - query predicate reverseRead(Node n, string msg) { - exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and - not any(ConsistencyConfiguration conf).reverseReadExclude(n) and - msg = "Origin of readStep is missing a PostUpdateNode." - } - - query predicate argHasPostUpdate(ArgumentNode n, string msg) { - not hasPost(n) and - not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and - msg = "ArgumentNode is missing PostUpdateNode." - } - - // This predicate helps the compiler forget that in some languages - // it is impossible for a `PostUpdateNode` to be the target of - // `simpleLocalFlowStep`. - private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() } - - query predicate postWithInFlow(Node n, string msg) { - isPostUpdateNode(n) and - not clearsContent(n, _) and - simpleLocalFlowStep(_, n) and - not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and - msg = "PostUpdateNode should not be the target of local flow." - } - - query predicate viableImplInCallContextTooLarge( - DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable - ) { - callable = viableImplInCallContext(call, ctx) and - not callable = viableCallable(call) and - not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable) - } - - query predicate uniqueParameterNodeAtPosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and - msg = "Parameters with overlapping positions." - } - - query predicate uniqueParameterNodePosition( - DataFlowCallable c, ParameterPosition pos, Node p, string msg - ) { - not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and - isParameterNode(p, c, pos) and - not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and - msg = "Parameter node with multiple positions." - } - - query predicate uniqueContentApprox(Content c, string msg) { - not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and - msg = "Non-unique content approximation." - } - - query predicate identityLocalStep(Node n, string msg) { - simpleLocalFlowStep(n, n) and - not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and - msg = "Node steps to itself" + predicate identityLocalStepExclude(Node n) { + not exists(n.getLocation().getFile().getRelativePath()) } } + +module Consistency = MakeConsistency; diff --git a/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll b/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll index fd56070f86b..76ddc3643f1 100644 --- a/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll +++ b/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll @@ -1,46 +1,6 @@ +// TODO: this should be promoted to be a REAL consistency query by being placed in +// `python/ql/consistency-queries`. For for now it resides here. import python import semmle.python.dataflow.new.DataFlow::DataFlow import semmle.python.dataflow.new.internal.DataFlowPrivate import semmle.python.dataflow.new.internal.DataFlowImplConsistency::Consistency - -// TODO: this should be promoted to be a REAL consistency query by being placed in -// `python/ql/consistency-queries`. For for now it resides here. -private class MyConsistencyConfiguration extends ConsistencyConfiguration { - override predicate argHasPostUpdateExclude(ArgumentNode n) { - exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isStarArgs(_)) - or - exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isDictSplat()) - } - - override predicate reverseReadExclude(Node n) { - // since `self`/`cls` parameters can be marked as implicit argument to `super()`, - // they will have PostUpdateNodes. We have a read-step from the synthetic `**kwargs` - // parameter, but dataflow-consistency queries should _not_ complain about there not - // being a post-update node for the synthetic `**kwargs` parameter. - n instanceof SynthDictSplatParameterNode - } - - override predicate uniqueParameterNodePositionExclude( - DataFlowCallable c, ParameterPosition pos, Node p - ) { - // For normal parameters that can both be passed as positional arguments or keyword - // arguments, we currently have parameter positions for both cases.. - // - // TODO: Figure out how bad breaking this consistency check is - exists(Function func, Parameter param | - c.getScope() = func and - p = parameterNode(param) and - c.getParameter(pos) = p and - param = func.getArg(_) and - param = func.getArgByName(_) - ) - } - - override predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { - not exists(call.getLocation().getFile().getRelativePath()) - } - - override predicate identityLocalStepExclude(Node n) { - not exists(n.getLocation().getFile().getRelativePath()) - } -}