Merge pull request #108 from joernio/claudiu/ghidra-env

Add getenvToStrcpy ghidra query

src/main/scala/io/joern/scanners/ghidra/MainArgsToStrcpy.scala → src/main/scala/io/joern/scanners/ghidra/UserInputIntoDangerousFunctions.scala

@@ -7,7 +7,7 @@ import io.shiftleft.semanticcpg.language._
 import io.shiftleft.dataflowengineoss.language._
 import io.shiftleft.dataflowengineoss.queryengine.EngineContext
 
-object MainArgsToStrcpy extends QueryBundle {
+object UserInputIntoDangerousFunctions extends QueryBundle {
 
   implicit val resolver: ICallResolver = NoResolve
 
@@ -24,9 +24,30 @@ object MainArgsToStrcpy extends QueryBundle {
       score = 4,
       withStrRep({ cpg =>
         def source = cpg.method.fullName("main").parameter
-        def sink = cpg.call.methodFullName("strcpy").argument
+        def sink = cpg.method.fullName("strcpy").parameter.index(2)
         sink.reachableBy(source).l
       }),
       tags = List(QueryTags.badfn)
     )
+
+  @q
+  def getenvToStrcpy()(implicit context: EngineContext): Query =
+    Query.make(
+      name = "getenv-to-strcpy",
+      author = Crew.claudiu,
+      title = "`getenv` fn arguments used in strcpy source buffer",
+      description =
+        """
+          |User-input ends up in source buffer argument of strcpy, which might overflow the destination buffer.
+          |""".stripMargin,
+      score = 4,
+      withStrRep({ cpg =>
+        def source =
+          cpg.call.methodFullName("getenv").cfgNext.isCall.argument(2)
+        def sink = cpg.method.fullName("strcpy").parameter.index(2)
+        sink.reachableBy(source).l
+      }),
+      tags = List(QueryTags.badfn)
+    )
+
 }

src/test/resources/testbinaries/buf2.c

@@ -0,0 +1,18 @@
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+// gcc -fno-stack-protector -z execstack -no-pie -o buf2 buf2.c
+int main(int argc, char *argv[]) {
+  const char* inEnv = getenv("BUF2IN");
+  if (inEnv == NULL) {
+    printf("BUF2IN environment variable not set.");
+    return -1;
+  }
+
+  char c[6];
+  strcpy(c, inEnv);
+  printf("First argument is: %s\n", c);
+  return 0;
+}
+

src/test/resources/testbinaries/buf2_neg.c

@@ -0,0 +1,18 @@
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+// gcc -fno-stack-protector -z execstack -no-pie -o buf2_neg buf2_neg.c
+int main(int argc, char *argv[]) {
+  const char* inEnv = getenv("BUF2IN");
+  if (inEnv == NULL) {
+    printf("BUF2IN environment variable not set.");
+    return -1;
+  }
+
+  char c[6];
+  strcpy(c, "NOTHING");
+  printf("First argument is: %s\n", c);
+  return 0;
+}
+

src/test/scala/io/joern/scanners/ghidra/UserInputIntoDangerousFunctionsTests.scala

@@ -0,0 +1,46 @@
+package io.joern.scanners.ghidra
+
+import io.joern.suites.GhidraQueryTestSuite
+
+class UserInputIntoDangerousFunctionsTests  extends GhidraQueryTestSuite {
+  override def queryBundle = UserInputIntoDangerousFunctions
+
+  "mainArgsToStrcpy query" when {
+    def query = queryBundle.mainArgsToStrcpy()
+    "executed on CPG for binary with dataflow between `main` fn args and `strcpy` source argument" should {
+      "find the `main` function among the tracking points returned" in {
+        buildCpgForBin("buf1.exe")
+        val results = methodNamesForMatchedPoints(query)
+        results shouldBe Set("main")
+      }
+    }
+  }
+
+  "getenvToStrcpy query" when {
+    def query = queryBundle.getenvToStrcpy()
+
+    "executed on CPG for binary call to `strcpy`, but no call to `getenv`" should {
+      "return an empty set of matched method names" in {
+        buildCpgForBin("buf1.exe")
+        val results = methodNamesForMatchedPoints(query)
+        results shouldBe Set()
+      }
+    }
+
+    "executed on CPG for binary call to `strcpy`, and call to `getenv`, but no dataflow between them" should {
+      "return an empty set of matched method names" in {
+        buildCpgForBin("buf2_neg.exe")
+        val results = methodNamesForMatchedPoints(query)
+        results shouldBe Set()
+      }
+    }
+
+    "executed on CPG for binary with dataflow between `getenv` return value and `strcpy` source argument" should {
+      "find main function with data flow between getenv and strcpy" in {
+        buildCpgForBin("buf2.exe")
+        val results = methodNamesForMatchedPoints(query)
+        results shouldBe Set("main")
+      }
+    }
+  }
+}

src/test/scala/io/joern/suites/GhidraQueryTestSuite.scala

@@ -8,6 +8,7 @@ import io.shiftleft.codepropertygraph.generated.nodes
 import io.shiftleft.console.{Query, QueryBundle}
 import io.shiftleft.utils.ProjectRoot
 import io.shiftleft.semanticcpg.language._
+import overflowdb.traversal.iterableToTraversal
 
 class GhidraQueryTestSuite extends DataFlowBinToCpgSuite {
   val argumentProvider = new QDBArgumentProvider(3)
@@ -22,21 +23,22 @@ class GhidraQueryTestSuite extends DataFlowBinToCpgSuite {
 
   def allQueries = QueryUtil.allQueries(queryBundle, argumentProvider)
 
-  def findMatchingMethodParam(query: Query): Set[String] = {
+  def findMatchingCalls(query: Query): Set[String] = {
     query(cpg)
       .flatMap(_.evidence)
-      .collect { case methodParam: nodes.MethodParameterIn => methodParam }
+      .collect { case call: nodes.Call => call }
       .method
       .name
       .toSetImmutable
   }
 
-  def findMatchingCalls(query: Query): Set[String] = {
+  def methodNamesForMatchedPoints(query: Query): Set[String] = {
+    nodes.MethodParameterIn
     query(cpg)
       .flatMap(_.evidence)
-      .collect { case call: nodes.Call => call }
-      .method
-      .name
+      .collect {
+        case cfgNode: nodes.CfgNode => cfgNode.method.name
+      }
       .toSetImmutable
   }
 }