A Foray into Go Source Code Review & CodeQL Queries

Recently I’ve been delving a bit deeper into Go and code review. While exploring Go through amazing books like “Let’s Go” and “Writing an Interpreter in Go” I’ve been inspired to soon start pursuing the OffSec OSWE certification. That’s a ways off, but I wanted to get started by looking at what others have already found and exploited in the Go project community. Let’s start with this one.

Authenticated Remote Command Execution

This was reported by JorgeCTF and was found by a CodeQL query that was made for Go named “Command built from user-controlled sources”. Before jumping too deep into breaking down the CodeQL query, take a look at the GitHub page to read what the issue was. It boils down to unsanitized user input being taken and included into a function that runs system commands. In Go, this looks like:

cmd := exec.Command("id")

In the above example, Go is executing the “id” command. Within the project that was affected, it appears like this:

func execShell(cmd string) (out string) {
	bytes, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput()
	out = string(bytes)
	if err != nil {
		out += " " + err.Error()
	}
	return
}

JorgeCTF made a PoC of them sending the request that executes this command here:

POST /api/settings HTTP/1.1
Host: 127.0.0.1:8080
Content-Length: 528
Authorization: <<JWT TOKEN>
Content-Type: application/json

[SNIP]
"test_config_cmd":"touch /tmp/pwned",
[SNIP]

They use the “touch” command to create a new directory “tmp/pwned”. I think this is another good example of a vulnerability that is both exploitable by a normal/less-privileged user and one that would be difficult to find in a run-of-the-mill penetration test. While a little more obvious in its parameter naming conventions that would draw the eye of any penetration tester (“test_config_cmd”), knowing that a new site would need to be added to trigger this, and also knowing that it would need to be a blind command injection, escalates the difficulty of being able to uncover this in a short pentest. Even if a scanner/fuzzer was left alone with this and all the potential requests for the application for a few weeks, it may not have the wherewithal to detect this. This highlights the benefits of static application security testing and code review.

Let’s dive into the CodeQL query now. This will be my first time really digging into them so please let me know if you notice a mistake (ha!).

Full Query:

/**
 * @name Command built from user-controlled sources
 * @description Building a system command from user-controlled sources is vulnerable to insertion of
 *              malicious code by the user.
 * @kind path-problem
 * @problem.severity error
 * @security-severity 9.8
 * @precision high
 * @id go/command-injection
 * @tags security
 *       external/cwe/cwe-078
 */

import go
import semmle.go.security.CommandInjection

module Flow =
  DataFlow::MergePathGraph<CommandInjection::Flow::PathNode,
    CommandInjection::DoubleDashSanitizingFlow::PathNode, CommandInjection::Flow::PathGraph,
    CommandInjection::DoubleDashSanitizingFlow::PathGraph>;

import Flow::PathGraph

from Flow::PathNode source, Flow::PathNode sink
where
  CommandInjection::Flow::flowPath(source.asPathNode1(), sink.asPathNode1()) or
  CommandInjection::DoubleDashSanitizingFlow::flowPath(source.asPathNode2(), sink.asPathNode2())
select sink.getNode(), source, sink, "This command depends on a $@.", source.getNode(),
  "user-provided value"

Starting at the top:

import go
import semmle.go.security.CommandInjection

The first line imports the CodeQL library for Go, while the second line imports a CoeQL library specifically made for detecting patterns for command injection vulnerabilities in Go.

module Flow

This is defining a new module named “Flow”, these are used to organize code and queries.

DataFlow::MergePathGraph

This is a CodeQL class that represents a data flow graph, AKA how data flows through a program. The “MergePathGraph” portion of it is a more specialized version of the graph that includes the ability to merge paths from different data flow graphs.

CommandInjection::Flow::PathNode,
CommandInjection::DoubleDashSanitizingFlow::PathNode

So, the data flow graph can have different types of path nodes in it. These path nodes represent a point in the program where data can flow from a “source” to a “sink”. If you’re more black-box, this brought me back to learning about DOM XSS. The first line is a general command injection flow, while the second one represents a case where command injection is prevented by using a double dash (--) as a sanitizer.

CommandInjection::Flow::PathGraph
CommandInjection::DoubleDashSanitizingFlow::PathGraph

You’ll notice that the only difference between the previous two and these two is that they use PathGraph instead of PathNode. So whereas the path node represents a point in a program where data can flow from a source to a sink, path graphs represent the actual paths of the data between the source to the sink.

where
  CommandInjection::Flow::flowPath(source.asPathNode1(), sink.asPathNode1()) or
  CommandInjection::DoubleDashSanitizingFlow::flowPath(source.asPathNode2(), sink.asPathNode2())

“where” is used here to specify the conditions of the “match”. In this instance, it will be checking if the paths follow either of the listed flows. The “flowPath” function checks if there is a path present from the source to a sink.

select sink.getNode(), source, sink, "This command depends on a $@.", source.getNode(),
  "user-provided value"

Here the “select” is the last part of the query that specifies which info to include in the results if a match is found. It will define the output of the query with information like the location of the issue and a message describing it. “sink.getNode()” is used to select the sink node from the data flow graph, which is the location of where the vulnerable command is executed. Last but not least, the “source.getNode()” is used to select the source where the user-provided input is received in the first place, which is represented by the source node.