Detect commands passed through simple launchers

Recognize fixed command positions without parsing launcher options. Preserve safe arguments after a known non-shell command.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: owen-mc

java/ql/lib/semmle/code/java/security/CommandArguments.qll

@@ -10,46 +10,110 @@ import semmle.code.java.Collections
 
 /**
  * Holds if `ex` is used safely as an argument to a command;
- * i.e. it's not in the first position and it's not a shell command.
+ * i.e. it's not in a position that selects a command to execute.
  */
 predicate isSafeCommandArgument(Expr ex) {
   exists(ArrayInit ai, int i |
     ex = ai.getInit(i) and
     i > 0 and
-    not isShell(ai.getInit(0))
+    not isShell(ai.getInit(0)) and
+    (
+      not exists(delegatedCommandIndex(ai.getInit(0)))
+      or
+      exists(int commandIndex |
+        commandIndex = delegatedCommandIndex(ai.getInit(0)) and
+        (
+          i < commandIndex
+          or
+          i > commandIndex and
+          not isShell(ai.getInit(commandIndex)) and
+          not exists(delegatedCommandIndex(ai.getInit(commandIndex)))
+        )
+      )
+    )
   )
   or
-  exists(CommandArgumentList cal |
+  exists(CommandArgumentList cal, int i |
     not cal.isShell() and
-    ex = cal.getASubsequentAdd().getArgument(0)
+    i > 0 and
+    ex = cal.getArgument(i) and
+    (
+      not exists(delegatedCommandIndex(cal.getArgument(0)))
+      or
+      exists(int commandIndex |
+        commandIndex = delegatedCommandIndex(cal.getArgument(0)) and
+        (
+          i < commandIndex
+          or
+          i > commandIndex and
+          exists(cal.getArgument(commandIndex)) and
+          not isShell(cal.getArgument(commandIndex)) and
+          not exists(delegatedCommandIndex(cal.getArgument(commandIndex)))
+        )
+      )
+    )
   )
   or
-  exists(CommandArgArrayImmutableFirst caa |
+  exists(CommandArgArrayImmutableFirst caa, int i |
     not caa.isShell() and
-    ex = caa.getAWrite(any(int i | i > 0))
+    i > 0 and
+    ex = caa.getAWrite(i) and
+    (
+      not exists(delegatedCommandIndex(caa.getFirstElement()))
+      or
+      exists(int commandIndex |
+        commandIndex = delegatedCommandIndex(caa.getFirstElement()) and
+        (
+          i < commandIndex
+          or
+          i > commandIndex and
+          exists(caa.getAnElement(commandIndex)) and
+          not isShell(caa.getAnElement(commandIndex)) and
+          not exists(delegatedCommandIndex(caa.getAnElement(commandIndex)))
+        )
+      )
+    )
   )
 }
 
-/**
- * Holds if the given expression is the name of a shell command such as bash or python
- */
-private predicate isShell(Expr ex) {
-  exists(string cmd | cmd = ex.(StringLiteral).getValue() |
-    cmd.regexpMatch(".*(sh|javac?|python[23]?|osascript|cmd)(\\.exe)?$")
-  )
+private string commandName(Expr ex) {
+  result = ex.(StringLiteral).getValue()
   or
   exists(SsaDefinition ssa, SsaExplicitWrite def |
     ex = ssa.getARead() and
-    isShell(def.getDefiningExpr())
+    result = commandName(def.getDefiningExpr())
   |
     ssa.getAnUltimateDefinition() = def
     or
     ssa.(SsaCapturedDefinition).getAnUltimateCapturedDefinition() = def
   )
   or
-  isShell(ex.(Assignment).getRhs())
+  result = commandName(ex.(Assignment).getRhs())
   or
-  isShell(ex.(LocalVariableDeclExpr).getInit())
+  result = commandName(ex.(LocalVariableDeclExpr).getInit())
+}
+
+/**
+ * Holds if the given expression is the name of a shell command such as bash or python
+ */
+private predicate isShell(Expr ex) {
+  commandName(ex).regexpMatch(".*(sh|javac?|python[23]?|osascript|cmd)(\\.exe)?$")
+}
+
+/**
+ * Gets the fixed position of the command executed by a supported delegating launcher.
+ *
+ * Only simple invocation forms are recognized. Launcher options are intentionally not parsed.
+ */
+private int delegatedCommandIndex(Expr ex) {
+  exists(string launcher |
+    launcher = commandName(ex).regexpCapture("(.*/)?([^/.]+)(\\.exe)?$", 2) and
+    (
+      launcher = ["env", "nice", "nohup", "setsid", "sudo", "xargs"] and result = 1
+      or
+      launcher = ["chroot", "stdbuf", "timeout"] and result = 2
+    )
+  )
 }
 
 /**
@@ -101,16 +165,28 @@ private class CommandArgumentList extends SsaExplicitWrite {
     result.getQualifier() = this.getAUseBeforeFirstAdd()
   }
 
-  /** Gets an addition to this list which is not the first element. */
-  MethodCall getASubsequentAdd() {
-    result = this.getAnAdd() and
-    not result = this.getAFirstAdd()
+  /** Gets a single-element addition at the given index. */
+  private MethodCall getAnAddAtIndex(int index) {
+    index = 0 and
+    result = this.getAFirstAdd() and
+    result.getMethod().hasName("add") and
+    result.getNumArgument() = 1
+    or
+    index > 0 and
+    exists(MethodCall previous |
+      previous = this.getAnAddAtIndex(index - 1) and
+      result = this.getAnAdd() and
+      result.getMethod().hasName("add") and
+      result.getNumArgument() = 1 and
+      adjacentUseUse(previous.getQualifier(), result.getQualifier())
+    )
   }
 
+  /** Gets the argument added at the given index. */
+  Expr getArgument(int index) { result = this.getAnAddAtIndex(index).getArgument(0) }
+
   /** Holds if the first element of this list is a shell command. */
-  predicate isShell() {
-    exists(MethodCall ma | ma = this.getAFirstAdd() and isShell(ma.getArgument(0)))
-  }
+  predicate isShell() { isShell(this.getArgument(0)) }
 }
 
 /**
@@ -153,7 +229,7 @@ private class CommandArgumentArray extends SsaExplicitWrite {
  */
 private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
   CommandArgArrayImmutableFirst() {
-    (exists(this.getAWrite(0)) or exists(firstElementOf(this.getDefiningExpr()))) and
+    (exists(this.getAWrite(0)) or exists(arrayElementOf(this.getDefiningExpr(), 0))) and
     forall(VarRead use | exists(this.getAWrite(0, use)) | use = ssaGetAFirstUse(this))
   }
 
@@ -162,41 +238,53 @@ private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
     result = this.getAWrite(0)
     or
     not exists(this.getAWrite(0)) and
-    result = firstElementOf(this.getDefiningExpr())
+    result = arrayElementOf(this.getDefiningExpr(), 0)
+  }
+
+  /** Gets a possible value for the element at the given index. */
+  Expr getAnElement(int index) {
+    index = 0 and result = this.getFirstElement()
+    or
+    index > 0 and
+    (
+      result = this.getAWrite(index)
+      or
+      result = arrayElementOf(this.getDefiningExpr(), index)
+    )
   }
 
   /** Holds if the first element of this array is a shell command. */
   predicate isShell() { isShell(this.getFirstElement()) }
 }
 
-/** Gets the first element of an imutable array of strings */
-private Expr firstElementOf(Expr arr) {
+/** Gets an element of an immutable array of strings. */
+private Expr arrayElementOf(Expr arr, int index) {
   arr.getType() instanceof ArrayOfStringType and
   (
-    result = firstElementOf(arr.(Assignment).getRhs())
+    result = arrayElementOf(arr.(Assignment).getRhs(), index)
     or
-    result = firstElementOf(arr.(LocalVariableDeclExpr).getInit())
+    result = arrayElementOf(arr.(LocalVariableDeclExpr).getInit(), index)
     or
     exists(CommandArgArrayImmutableFirst caa | arr = caa.getARead() |
-      result = caa.getFirstElement()
+      result = caa.getAnElement(index)
     )
     or
     exists(MethodCall ma, Method m |
       arr = ma and
       ma.getMethod() = m and
       m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
       m.hasName("copyOf") and
-      result = firstElementOf(ma.getArgument(0))
+      result = arrayElementOf(ma.getArgument(0), index)
     )
     or
     exists(Field f |
       f.isStatic() and
       arr.(FieldRead).getField() = f and
-      result = firstElementOf(f.getInitializer())
+      result = arrayElementOf(f.getInitializer(), index)
     )
     or
-    result = arr.(ArrayInit).getInit(0)
+    result = arr.(ArrayInit).getInit(index)
     or
-    result = arr.(ArrayCreationExpr).getInit().getInit(0)
+    result = arr.(ArrayCreationExpr).getInit().getInit(index)
   )
 }