C++: Refactor models to prevent IR reevaluation

Author: rdmarsh2

cpp/ql/lib/semmle/code/cpp/models/Models.qll

@@ -33,3 +33,4 @@ private import implementations.Recv
 private import implementations.Accept
 private import implementations.Poll
 private import implementations.Select
+private import implementations.System

cpp/ql/lib/semmle/code/cpp/models/implementations/System.qll

@@ -0,0 +1,44 @@
+import cpp
+import semmle.code.cpp.security.FunctionWithWrappers
+import semmle.code.cpp.models.interfaces.SideEffect
+import semmle.code.cpp.models.interfaces.Alias
+import semmle.code.cpp.models.interfaces.CommandExecution
+
+/**
+ * A function for running a command using a command interpreter.
+ */
+private class SystemFunction extends CommandExecutionFunction, ArrayFunction, AliasFunction,
+  SideEffectFunction {
+  SystemFunction() {
+    hasGlobalOrStdName("system") or // system(command)
+    hasGlobalName("popen") or // popen(command, mode)
+    // Windows variants
+    hasGlobalName("_popen") or // _popen(command, mode)
+    hasGlobalName("_wpopen") or // _wpopen(command, mode)
+    hasGlobalName("_wsystem") // _wsystem(command)
+  }
+
+  override predicate hasCommandArgument(FunctionInput input) { input.isParameterDeref(0) }
+
+  override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 or bufParam = 1 }
+
+  override predicate hasArrayInput(int bufParam) { bufParam = 0 or bufParam = 1 }
+
+  override predicate parameterNeverEscapes(int index) { index = 0 or index = 1 }
+
+  override predicate parameterEscapesOnlyViaReturn(int index) { none() }
+
+  override predicate parameterIsAlwaysReturned(int index) { none() }
+
+  override predicate hasOnlySpecificReadSideEffects() { any() }
+
+  override predicate hasOnlySpecificWriteSideEffects() {
+    hasGlobalOrStdName("system") or
+    hasGlobalName("_wsystem")
+  }
+
+  override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
+    (i = 0 or i = 1) and
+    buffer = true
+  }
+}

cpp/ql/lib/semmle/code/cpp/models/interfaces/CommandExecution.qll

@@ -0,0 +1,7 @@
+import cpp
+import FunctionInputsAndOutputs
+import semmle.code.cpp.models.Models
+
+abstract class CommandExecutionFunction extends Function {
+  abstract predicate hasCommandArgument(FunctionInput input);
+}

cpp/ql/lib/semmle/code/cpp/security/CommandExecution.qll

@@ -4,42 +4,17 @@ import cpp
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.models.interfaces.SideEffect
 import semmle.code.cpp.models.interfaces.Alias
+import semmle.code.cpp.models.interfaces.CommandExecution
 
-/**
- * A function for running a command using a command interpreter.
- */
-class SystemFunction extends FunctionWithWrappers, ArrayFunction, AliasFunction, SideEffectFunction {
-  SystemFunction() {
-    hasGlobalOrStdName("system") or // system(command)
-    hasGlobalName("popen") or // popen(command, mode)
-    // Windows variants
-    hasGlobalName("_popen") or // _popen(command, mode)
-    hasGlobalName("_wpopen") or // _wpopen(command, mode)
-    hasGlobalName("_wsystem") // _wsystem(command)
-  }
-
-  override predicate interestingArg(int arg) { arg = 0 }
-
-  override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 or bufParam = 1 }
-
-  override predicate hasArrayInput(int bufParam) { bufParam = 0 or bufParam = 1 }
-
-  override predicate parameterNeverEscapes(int index) { index = 0 or index = 1 }
-
-  override predicate parameterEscapesOnlyViaReturn(int index) { none() }
-
-  override predicate parameterIsAlwaysReturned(int index) { none() }
-
-  override predicate hasOnlySpecificReadSideEffects() { any() }
-
-  override predicate hasOnlySpecificWriteSideEffects() {
-    hasGlobalOrStdName("system") or
-    hasGlobalName("_wsystem")
-  }
-
-  override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
-    (i = 0 or i = 1) and
-    buffer = true
+class WrappedSystemFunction extends FunctionWithWrappers instanceof CommandExecutionFunction {
+  override predicate interestingArg(int arg) {
+    exists(FunctionInput input |
+      this.(CommandExecutionFunction).hasCommandArgument(input) and
+      (
+        input.isParameterDerefOrQualifierObject(arg) or
+        input.isParameterOrQualifierAddress(arg)
+      )
+    )
   }
 }
 
@@ -185,7 +160,7 @@ predicate shellCommandPreface(string cmd, string flag) {
  */
 predicate shellCommand(Expr command, string callChain) {
   // A call to a function like system()
-  exists(SystemFunction systemFunction |
+  exists(WrappedSystemFunction systemFunction |
     systemFunction.outermostWrapperFunctionCall(command, callChain)
   )
   or