diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll index 0d391d7f98d7..d232827a247e 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll @@ -42,15 +42,27 @@ private import semmle.python.internal.CachedStages newtype TParameterPosition = /** Used for `self` in methods, and `cls` in classmethods. */ TSelfParameterPosition() or - TPositionalParameterPosition(int index) { - index = any(Parameter p).getPosition() + TNormalParameterPosition(int index, string name) { + exists(Function f, Parameter p, int indexOffset | + p = f.getArg(index + indexOffset) and + p = f.getArgByName(name) and + indexOffset = any(DataFlowFunction dff | dff.getScope() = f).positionalOffset() and + // for things that will be considered a self parameter, we do not need to create a + // parameter position + index >= 0 + ) + } or + TPositionalOnlyParameterPosition(int index) { + index = any(Parameter p).getPosition() and + // TODO: implement this when we have proper support for positional only + none() or // since synthetic parameters are made for a synthetic summary callable, based on // what Argument positions they have flow for, we need to make sure we have such // parameter positions available. FlowSummaryImplSpecific::ParsePositions::isParsedPositionalArgumentPosition(_, index) } or - TKeywordParameterPosition(string name) { + TKeywordOnlyParameterPosition(string name) { name = any(Parameter p).getName() or // see comment for TPositionalParameterPosition @@ -71,11 +83,14 @@ class ParameterPosition extends TParameterPosition { /** Holds if this position represents a `self`/`cls` parameter. */ predicate isSelf() { this = TSelfParameterPosition() } - /** Holds if this position represents a positional parameter at (0-based) `index`. */ - predicate isPositional(int index) { this = TPositionalParameterPosition(index) } + /** Holds if this position represents a normal parameter at (0-based) `index` with name `name`. */ + predicate isNormal(int index, string name) { this = TNormalParameterPosition(index, name) } + + /** Holds if this position represents a positional only parameter at (0-based) `index`. */ + predicate isPositionalOnly(int index) { this = TPositionalOnlyParameterPosition(index) } - /** Holds if this position represents a keyword parameter named `name`. */ - predicate isKeyword(string name) { this = TKeywordParameterPosition(name) } + /** Holds if this position represents a keyword only parameter named `name`. */ + predicate isKeywordOnly(string name) { this = TKeywordOnlyParameterPosition(name) } /** Holds if this position represents a `*args` parameter at (0-based) `index`. */ predicate isStarArgs(int index) { this = TStarArgsParameterPosition(index) } @@ -96,9 +111,13 @@ class ParameterPosition extends TParameterPosition { string toString() { this.isSelf() and result = "self" or - exists(int index | this.isPositional(index) and result = "position " + index) + exists(int index, string name | + this.isNormal(index, name) and result = "positional " + index + " or keyword " + name + ) or - exists(string name | this.isKeyword(name) and result = "keyword " + name) + exists(int index | this.isPositionalOnly(index) and result = "positional only " + index) + or + exists(string name | this.isKeywordOnly(name) and result = "keyword only " + name) or exists(int index | this.isStarArgs(index) and result = "*args at " + index) or @@ -168,9 +187,11 @@ class ArgumentPosition extends TArgumentPosition { predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos.isSelf() and apos.isSelf() or - exists(int index | ppos.isPositional(index) and apos.isPositional(index)) + exists(int index | apos.isPositional(index) | + ppos.isNormal(index, _) or ppos.isPositionalOnly(index) + ) or - exists(string name | ppos.isKeyword(name) and apos.isKeyword(name)) + exists(string name | apos.isKeyword(name) | ppos.isNormal(_, name) or ppos.isKeywordOnly(name)) or exists(int index | ppos.isStarArgs(index) and apos.isStarArgs(index)) or @@ -297,11 +318,23 @@ abstract class DataFlowFunction extends DataFlowCallable, TFunction { int positionalOffset() { result = 0 } override ParameterNode getParameter(ParameterPosition ppos) { - exists(int index | ppos.isPositional(index) | - result.getParameter() = func.getArg(index + this.positionalOffset()) + // normal + exists(int index, string name | ppos.isNormal(index, name) | + result.getParameter() = func.getArg(index + this.positionalOffset()) and + func.getArg(index + this.positionalOffset()) = func.getArgByName(name) ) or - exists(string name | ppos.isKeyword(name) | result.getParameter() = func.getArgByName(name)) + // positioanl only + exists(int index | ppos.isPositionalOnly(index) | + result.getParameter() = func.getArg(index + this.positionalOffset()) and + not func.getArg(index + this.positionalOffset()) = func.getArgByName(_) + ) + or + // keyword only + exists(string name | ppos.isKeywordOnly(name) | + result.getParameter() = func.getArgByName(name) and + not func.getArgByName(name) = func.getArg(_) + ) or // `*args` exists(int index | @@ -1467,7 +1500,7 @@ class SummaryParameterNode extends ParameterNodeImpl, TSummaryParameterNode { pos.isDictSplat() and exists(ParameterPosition keywordPos | FlowSummaryImpl::Private::summaryParameterNodeRange(sc, keywordPos) and - keywordPos.isKeyword(_) + (keywordPos.isNormal(_, _) or keywordPos.isKeywordOnly(_)) ) ) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index 7b3234bfedb9..04ca4a6c1e14 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -188,7 +188,11 @@ private predicate dictSplatParameterNodeClearStep(ParameterNode n, DictionaryEle n = TSummaryParameterNode(callable.asLibraryCallable(), dictSplatPos) ) and exists(callable.getParameter(keywordPos)) and - keywordPos.isKeyword(c.getKey()) + ( + keywordPos.isNormal(_, c.getKey()) + or + keywordPos.isKeywordOnly(c.getKey()) + ) ) } @@ -268,7 +272,11 @@ predicate synthDictSplatParameterNodeReadStep( exists(DataFlowCallable callable, ParameterPosition ppos | nodeFrom = TSynthDictSplatParameterNode(callable) and nodeTo = callable.getParameter(ppos) and - ppos.isKeyword(c.getKey()) + ( + ppos.isNormal(_, c.getKey()) + or + ppos.isKeywordOnly(c.getKey()) + ) ) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll index 440ce3b70d4c..1777c3832c99 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll @@ -121,7 +121,9 @@ newtype TNode = TSynthDictSplatArgumentNode(CallNode call) { exists(call.getArgByName(_)) } or /** A synthetic node to allow flow to keyword parameters from a `**kwargs` argument. */ TSynthDictSplatParameterNode(DataFlowCallable callable) { - exists(ParameterPosition ppos | ppos.isKeyword(_) | exists(callable.getParameter(ppos))) + exists(ParameterPosition ppos | ppos.isNormal(_, _) or ppos.isKeywordOnly(_) | + exists(callable.getParameter(ppos)) + ) } /** Helper for `Node::getEnclosingCallable`. */ diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImplSpecific.qll b/python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImplSpecific.qll index e2308a22e749..8e4a3e23e647 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImplSpecific.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImplSpecific.qll @@ -118,12 +118,20 @@ string getParameterPosition(ParameterPosition pos) { pos.isSelf() and result = "self" or exists(int i | - pos.isPositional(i) and + ( + pos.isNormal(i, _) + or + pos.isPositionalOnly(i) + ) and result = i.toString() ) or exists(string name | - pos.isKeyword(name) and + ( + pos.isNormal(_, name) + or + pos.isKeywordOnly(name) + ) and result = name + ":" ) } @@ -262,12 +270,12 @@ ArgumentPosition parseParamBody(string s) { ParameterPosition parseArgBody(string s) { exists(int i | ParsePositions::isParsedPositionalArgumentPosition(s, i) and - result.isPositional(i) + result.isPositionalOnly(i) ) or exists(string name | ParsePositions::isParsedKeywordArgumentPosition(s, name) and - result.isKeyword(name) + result.isKeywordOnly(name) ) or s = "self" and diff --git a/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll b/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll index 456b18e2a87c..4b1a5e966a08 100644 --- a/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll +++ b/python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll @@ -28,20 +28,4 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration { pos.isDictSplat() and not exists(p.getLocation().getFile().getRelativePath()) } - - 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(_) - ) - } } diff --git a/python/ql/test/experimental/dataflow/basic/callGraphSinks.expected b/python/ql/test/experimental/dataflow/basic/callGraphSinks.expected index 0f87376ef1a0..5757dd1174cc 100644 --- a/python/ql/test/experimental/dataflow/basic/callGraphSinks.expected +++ b/python/ql/test/experimental/dataflow/basic/callGraphSinks.expected @@ -1,4 +1,4 @@ -| file://:0:0:0:0 | parameter position 0 of builtins.reversed | +| file://:0:0:0:0 | parameter positional only 0 of builtins.reversed | | test.py:1:1:1:21 | SynthDictSplatParameterNode | | test.py:1:19:1:19 | ControlFlowNode for x | | test.py:7:5:7:20 | ControlFlowNode for obfuscated_id() | diff --git a/python/ql/test/experimental/dataflow/basic/global.expected b/python/ql/test/experimental/dataflow/basic/global.expected index 800312b07be3..b8a731d51507 100644 --- a/python/ql/test/experimental/dataflow/basic/global.expected +++ b/python/ql/test/experimental/dataflow/basic/global.expected @@ -1,4 +1,4 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | | test.py:1:1:1:21 | ControlFlowNode for FunctionExpr | test.py:1:5:1:17 | GSSA Variable obfuscated_id | | test.py:1:1:1:21 | ControlFlowNode for FunctionExpr | test.py:7:5:7:17 | ControlFlowNode for obfuscated_id | | test.py:1:5:1:17 | GSSA Variable obfuscated_id | test.py:7:5:7:17 | ControlFlowNode for obfuscated_id | diff --git a/python/ql/test/experimental/dataflow/basic/globalStep.expected b/python/ql/test/experimental/dataflow/basic/globalStep.expected index fa5b20486c2f..08494c47b290 100644 --- a/python/ql/test/experimental/dataflow/basic/globalStep.expected +++ b/python/ql/test/experimental/dataflow/basic/globalStep.expected @@ -1,4 +1,4 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | | test.py:1:1:1:21 | ControlFlowNode for FunctionExpr | test.py:1:5:1:17 | GSSA Variable obfuscated_id | | test.py:1:1:1:21 | ControlFlowNode for FunctionExpr | test.py:1:5:1:17 | GSSA Variable obfuscated_id | | test.py:1:1:1:21 | ControlFlowNode for FunctionExpr | test.py:7:5:7:17 | ControlFlowNode for obfuscated_id | diff --git a/python/ql/test/experimental/dataflow/basic/local.expected b/python/ql/test/experimental/dataflow/basic/local.expected index 2354efea8e5a..4c86a9a3d639 100644 --- a/python/ql/test/experimental/dataflow/basic/local.expected +++ b/python/ql/test/experimental/dataflow/basic/local.expected @@ -1,8 +1,8 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | | file://:0:0:0:0 | [summary] to write: return (return) in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return) in builtins.reversed | | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | -| file://:0:0:0:0 | parameter position 0 of builtins.reversed | file://:0:0:0:0 | parameter position 0 of builtins.reversed | +| file://:0:0:0:0 | parameter positional only 0 of builtins.reversed | file://:0:0:0:0 | parameter positional only 0 of builtins.reversed | | test.py:0:0:0:0 | GSSA Variable __name__ | test.py:0:0:0:0 | GSSA Variable __name__ | | test.py:0:0:0:0 | GSSA Variable __package__ | test.py:0:0:0:0 | GSSA Variable __package__ | | test.py:0:0:0:0 | GSSA Variable b | test.py:0:0:0:0 | GSSA Variable b | diff --git a/python/ql/test/experimental/dataflow/basic/localStep.expected b/python/ql/test/experimental/dataflow/basic/localStep.expected index 534c31da1a6c..98f99cb8ff5d 100644 --- a/python/ql/test/experimental/dataflow/basic/localStep.expected +++ b/python/ql/test/experimental/dataflow/basic/localStep.expected @@ -1,4 +1,4 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | | test.py:1:1:1:21 | ControlFlowNode for FunctionExpr | test.py:1:5:1:17 | GSSA Variable obfuscated_id | | test.py:1:5:1:17 | GSSA Variable obfuscated_id | test.py:7:5:7:17 | ControlFlowNode for obfuscated_id | | test.py:1:19:1:19 | ControlFlowNode for x | test.py:1:19:1:19 | SSA variable x | diff --git a/python/ql/test/experimental/dataflow/basic/sinks.expected b/python/ql/test/experimental/dataflow/basic/sinks.expected index aafff76bbe2b..6af606318b93 100644 --- a/python/ql/test/experimental/dataflow/basic/sinks.expected +++ b/python/ql/test/experimental/dataflow/basic/sinks.expected @@ -1,7 +1,7 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | | file://:0:0:0:0 | [summary] to write: return (return) in builtins.reversed | | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | -| file://:0:0:0:0 | parameter position 0 of builtins.reversed | +| file://:0:0:0:0 | parameter positional only 0 of builtins.reversed | | test.py:0:0:0:0 | GSSA Variable __name__ | | test.py:0:0:0:0 | GSSA Variable __package__ | | test.py:0:0:0:0 | GSSA Variable b | diff --git a/python/ql/test/experimental/dataflow/basic/sources.expected b/python/ql/test/experimental/dataflow/basic/sources.expected index aafff76bbe2b..6af606318b93 100644 --- a/python/ql/test/experimental/dataflow/basic/sources.expected +++ b/python/ql/test/experimental/dataflow/basic/sources.expected @@ -1,7 +1,7 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | | file://:0:0:0:0 | [summary] to write: return (return) in builtins.reversed | | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | -| file://:0:0:0:0 | parameter position 0 of builtins.reversed | +| file://:0:0:0:0 | parameter positional only 0 of builtins.reversed | | test.py:0:0:0:0 | GSSA Variable __name__ | | test.py:0:0:0:0 | GSSA Variable __package__ | | test.py:0:0:0:0 | GSSA Variable b | diff --git a/python/ql/test/experimental/dataflow/tainttracking/basic/LocalTaintStep.expected b/python/ql/test/experimental/dataflow/tainttracking/basic/LocalTaintStep.expected index 05b64297f71a..790f84e18447 100644 --- a/python/ql/test/experimental/dataflow/tainttracking/basic/LocalTaintStep.expected +++ b/python/ql/test/experimental/dataflow/tainttracking/basic/LocalTaintStep.expected @@ -1,4 +1,4 @@ -| file://:0:0:0:0 | [summary] read: argument position 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | +| file://:0:0:0:0 | [summary] read: argument positional only 0.List element in builtins.reversed | file://:0:0:0:0 | [summary] to write: return (return).List element in builtins.reversed | | test.py:3:1:3:7 | GSSA Variable tainted | test.py:4:6:4:12 | ControlFlowNode for tainted | | test.py:3:11:3:16 | ControlFlowNode for SOURCE | test.py:3:1:3:7 | GSSA Variable tainted | | test.py:6:1:6:11 | ControlFlowNode for FunctionExpr | test.py:6:5:6:8 | GSSA Variable func |