add minimal test for Array join as a sink, and learn that the order is flipped compared to JS. Thanks Copilot!

Author: erik-krogh

python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -101,7 +101,6 @@ module UnsafeShellCommandConstruction {
    * A string constructed using a `.join(" ")` call, where the resulting string ends up being executed as a shell command.
    */
   class ArrayJoin extends Sink {
-    // TODO: Add test.
     Concepts::SystemCommandExecution s;
     DataFlow::MethodCallNode call;
 
@@ -110,10 +109,10 @@ module UnsafeShellCommandConstruction {
       unique( | | call.getArg(_)).asExpr().(Str).getText() = " " and
       isUsedAsShellCommand(call, s) and
       (
-        this = call.getObject() and
-        not call.getObject().asExpr() instanceof List
+        this = call.getArg(0) and
+        not call.getArg(0).asExpr() instanceof List
         or
-        this.asExpr() = call.getObject().asExpr().(List).getASubExpression()
+        this.asExpr() = call.getArg(0).asExpr().(List).getASubExpression()
       )
     }
 

python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected

@@ -1,11 +1,14 @@
 edges
 | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name |
 | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:8:23:8:26 | ControlFlowNode for name |
+| src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:11:25:11:38 | ControlFlowNode for Attribute() |
 nodes
 | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
 | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
 | src/unsafe_shell_test.py:8:23:8:26 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
+| src/unsafe_shell_test.py:11:25:11:38 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
 subpaths
 #select
 | src/unsafe_shell_test.py:5:15:5:28 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:5:25:5:28 | ControlFlowNode for name | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:5:5:5:29 | ControlFlowNode for Attribute() | shell command |
 | src/unsafe_shell_test.py:8:15:8:28 | ControlFlowNode for Fstring | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:8:23:8:26 | ControlFlowNode for name | This string construction which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:8:5:8:29 | ControlFlowNode for Attribute() | shell command |
+| src/unsafe_shell_test.py:11:15:11:38 | ControlFlowNode for BinaryExpr | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | src/unsafe_shell_test.py:11:25:11:38 | ControlFlowNode for Attribute() | This string concatenation which depends on $@ is later used in a $@. | src/unsafe_shell_test.py:4:22:4:25 | ControlFlowNode for name | library input | src/unsafe_shell_test.py:11:5:11:39 | ControlFlowNode for Attribute() | shell command |

python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/src/unsafe_shell_test.py

@@ -6,3 +6,6 @@ def unsafe_shell_one(name):
 
     # f-strings
     os.system(f"ping {name}") # $result=BAD
+
+    # array.join
+    os.system("ping " + " ".join(name)) # $result=BAD