Addressing comments we missed earlier

Author: raulgarciamsft

csharp/ql/src/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.ql

@@ -36,11 +36,10 @@ predicate isGetHash(Expr arg) {
     mc.getAnArgument() = arg
   )
   or
-  exists(Callable callable, Parameter param, Call call, int i |
+  exists(Callable callable, Parameter param, Call call |
     isCallableAPotentialNonCryptographicHashFunction(callable, param) and
-    callable.getParameter(i) = param and
     call = callable.getACall() and
-    call.getArgument(i) = arg
+    arg = call.getArgumentForParameter(param)
   )
 }
 

csharp/ql/src/experimental/Security Features/campaign/Solorigate/NumberOfKnownMethodNamesAboveThreshold.qhelp

@@ -4,7 +4,7 @@
 <qhelp>
   <overview>
     <p>This query detects method names used in the Solorigate code.</p>
-    <p>This query detects when the code includes at least 50 of the mthods used in the Solorigate tampered code</p>
+    <p>This query detects when the code includes at least 50 of the methods used in the Solorigate tampered code</p>
     <p>Please notice that by themselves these method names are not malign.</p>
   </overview>
 

csharp/ql/src/experimental/Security Features/campaign/Solorigate/NumberOfKnownMethodNamesAboveThreshold.ql

@@ -18,8 +18,7 @@ import Solorigate
  */
 
 int countSolorigateSuspiciousMethodNames() {
-  result =
-    count(string s | exists(Method m | s = m.getName() and s = solorigateSuspiciousMethodNames()))
+  result = count(string s | s = any(Method m | isSolorigateSuspiciousMethodName(m)).getName())
 }
 
 from Method m, int total, int threshold

csharp/ql/src/experimental/code/csharp/Cryptography/NonCryptographicHashes.qll

@@ -23,17 +23,15 @@ predicate maybeANonCryptogrphicHash(Callable callable, Variable v, Expr xor, Exp
  * where there is a loop statement `loop` where the variable `v` is used in an xor `xor` expression
  * followed by a multiplication `mul` expression.
  */
-predicate maybeUsedInFNVFunction(Variable v, Expr xor, Expr mul, LoopStmt loop) {
+predicate maybeUsedInFNVFunction(Variable v, Operation xor, Operation mul, LoopStmt loop) {
   exists(Expr e1, Expr e2 |
-    exists(Operation axore, Operation amule | xor = axore and mul = amule |
-      e1.getAChild*() = v.getAnAccess() and
-      e2.getAChild*() = v.getAnAccess() and
-      e1 = axore.getAnOperand() and
-      e2 = amule.getAnOperand() and
-      axore.getAControlFlowNode().getASuccessor*() = amule.getAControlFlowNode() and
-      (axore instanceof AssignXorExpr or axore instanceof BitwiseXorExpr) and
-      (amule instanceof AssignMulExpr or amule instanceof MulExpr)
-    )
+    e1.getAChild*() = v.getAnAccess() and
+    e2.getAChild*() = v.getAnAccess() and
+    e1 = xor.getAnOperand() and
+    e2 = mul.getAnOperand() and
+    xor.getAControlFlowNode().getASuccessor*() = mul.getAControlFlowNode() and
+    (xor instanceof AssignXorExpr or xor instanceof BitwiseXorExpr) and
+    (mul instanceof AssignMulExpr or mul instanceof MulExpr)
   ) and
   loop.getAChild*() = mul.getEnclosingStmt() and
   loop.getAChild*() = xor.getEnclosingStmt()
@@ -44,13 +42,11 @@ predicate maybeUsedInFNVFunction(Variable v, Expr xor, Expr mul, LoopStmt loop)
  * where there is a loop statement `loop` where the variable `v` is used in an xor `xor` expression
  * followed by an addition `add` expression.
  */
-predicate maybeUsedInElfHashFunction(Variable v, Expr xorExpr, Expr addExpr, LoopStmt loop) {
+private predicate maybeUsedInElfHashFunction(Variable v, Operation xor, Operation add, LoopStmt loop) {
   exists(
-    Expr e1, Operation add, Expr e2, AssignExpr addAssign, Operation xor, AssignExpr xorAssign,
-    Operation notOp, AssignExpr notAssign
+    Expr e1, Expr e2, AssignExpr addAssign, AssignExpr xorAssign, Operation notOp,
+    AssignExpr notAssign
   |
-    xorExpr = xor and
-    addExpr = add and
     (add instanceof AddExpr or add instanceof AssignAddExpr) and
     e1.getAChild*() = add.getAnOperand() and
     e1 instanceof BinaryBitwiseOperation and
@@ -70,17 +66,6 @@ predicate maybeUsedInElfHashFunction(Variable v, Expr xorExpr, Expr addExpr, Loo
   )
 }
 
-/**
- * Any dataflow from any source to any sink, used internally
- */
-private class AnyDataFlow extends TaintTracking2::Configuration {
-  AnyDataFlow() { this = "DataFlowFromDataGatheringMethodToVariable" }
-
-  override predicate isSource(Node source) { any() }
-
-  override predicate isSink(Node sink) { any() }
-}
-
 /**
  * Holds if the Callable is a function that behaves like a non-cryptographic hash
  * where the parameter `param` is likely the message to hash
@@ -94,13 +79,13 @@ predicate isCallableAPotentialNonCryptographicHashFunction(Callable callable, Pa
       or
       param.getAnAccess() = op2.(Operation).getAnOperand().getAChild*()
       or
-      exists(AnyDataFlow config, Node source, Node sink |
+      exists(Node source, Node sink |
         (
           sink.asExpr() = op1.(Operation).getAChild*() or
           sink.asExpr() = op2.(Operation).getAChild*()
         ) and
         source.asExpr() = param.getAnAccess() and
-        config.hasFlow(source, sink)
+        DataFlow::localFlow(source, sink)
       )
     )
   )