CodeQL - Exploring the Terrain #1

By playing CodeQL CTF: Go and don’t return organized by GitHub Security Lab.

For some reason, CodeQL simultaneously feels like it has tons of documentation and none at all. In all honestly, I am really having a hard time writing customized queries because I, for the love of God, cannot find simple examples that hints at how one could write such queries for bug hunting.

It’s either I am not looking at the right set of documentation, or I am just too stupid to understand them – the story of my life 😩 – either way, here’s my attempt at learning the basics of writing queries through CodeQL CTF: Go and don’t return by breaking down the reference answer.

Note that this is an evolving post which will be updated as I work my way through the exercises. Also, a word of warning, explanations in this post may not make sense to anyone but myself. I probably got most of the terminology wrong too, oh well 💁‍♀️

Setup Instructions

Paraphrasing from the official instructions, here’s the gist of what you need to do:

  1. Install Visual Studio Code
  2. Install the CodeQL extension for Visual Studio Code
  3. Clone https://github.com/github/vscode-codeql-starter/ with git clone --recursive
  4. In VS Code, click File > Open Workspace. Select the file vscode-codeql-starter.code-workspace in your checkout of this repository
  5. Download CodeQL database for MinIO and import it into VS Code by:
    • Opening the CodeQL Databases view in the sidebar
    • Chosing to add a database from a local ZIP archive with the zip file downloaded
  6. Test by running the example.ql query that is in the codeql-custom-queries-go folder

Got some results? Alright, let’s start “playing” – ahem, copying answers 🙈

The Challenge

The challenge requires one to leverage on CodeQL to write a series of queries to find unsafely implemented code that mostly revolves around the below snippet which can be found in full at this fix commit:

func validateAdminSignature(ctx context.Context, r *http.Request, region string) (auth.Credentials, map[string]interface{}, bool, APIErrorCode) {
      ...
      s3Err = isReqAuthenticated(ctx, r, region, serviceS3)
  }

  if s3Err != ErrNone {
      reqInfo := (&logger.ReqInfo{}).AppendTags("requestHeaders", dumpRequest(r))
      ctx := logger.SetReqInfo(ctx, reqInfo)
      logger.LogIf(ctx, errors.New(getAPIError(s3Err).Description), logger.Application)
      // missing return statement here that triggered the vulnerability
  }
      ...
}

Step 1.1: Finding references to ErrNone

Find all variables named “ErrNone”

from Ident i
where i.getName() = "ErrNone"
select i

Step 1.2: Finding equality tests against ErrNone

Find all operands (variable) that compares against “ErrNone”

from EqualityTestExpr eq
where eq.getAnOperand().(Ident).getName() = "ErrNone"
select eq

Step 1.3: Finding if-blocks making such a test

Find all if-statements that compares against “ErrNone”

AKA, we are only finding = <variable_called_ErrNone>

from IfStmt i
where i.getCond().(EqualityTestExpr).getAnOperand().(Ident).getName() = "ErrNone"
select i

Step 1.4: Finding return statements

Find all return statements

from ReturnStmt r
select r

Step 1.5: Finding if-blocks without return statements

Find all if-blocks that don’t contain return statements in their then branch

from IfStmt i
where not i.getThen().getAStmt() instanceof ReturnStmt
select i

Step 1.6: Putting it all together

Find the if-blocks testing for equality to ErrNone with no return

from IfStmt i
where
  i.getCond().(EqualityTestExpr).getAnOperand().(Ident).getName() = "ErrNone" and
  not i.getThen().getAStmt() instanceof ReturnStmt
select i

Step 2.1: Find conditionals that are fed from calls to isReqAuthenticated

Find all equality tests of DataFlow::EqualityTestNode type where the operand is a sink of a data-flow configuration that tracks data flowing from ANY call –> into isReqAuthenticated() –> ANY equality test operand

First, create customized class to override default source and sinks in a data-flow via DataFlow::Configuration where:

class CustomizedAuth extends DataFlow::Configuration {
  // fix for compiler that complains "this" is not binded
  CustomizedAuth() { this = "random-string" }

  // define source as any method call of minio entity type and supplied variable name
  override predicate isSource(DataFlow::Node source) {
    source =
      any(DataFlow::CallNode cn |
        cn.getTarget().hasQualifiedName("github.com/minio/minio/cmd", "isReqAuthenticated")
      ).getResult()
  }

  // define sink as operand of any DataFlow::EqualityTestNode found in data-flow
  override predicate isSink(DataFlow::Node sink) {
    sink = any(DataFlow::EqualityTestNode n).getAnOperand()
  }
}

With customized class defined, to find the desired data-flow:

AKA, we should first define the source and sinks based on requirements. After all relevant data-flow are identified, select flows (via the sink variable) that has similar operand as supplied node.

from CustomizedAuth config, DataFlow::Node sink, DataFlow::EqualityTestNode compare
where config.hasFlow(_, sink) and compare.getAnOperand() = sink
select compare

Step 2.2: Find the true bug!

Find all if-statements:

To fulfill the first condition, we can build upon the query we have in [Step 2.1] by converting it into a predicate/function (no idea what is the proper term here) that could be reused.

--  Before
from CustomizedAuth config, DataFlow::Node sink, DataFlow::EqualityTestNode compare
where config.hasFlow(_, sink) and compare.getAnOperand() = sink
select compare
// After
EqualityTestExpr checkAuth() {
  exists(CustomizedAuth config, DataFlow::Node sink, DataFlow::EqualityTestNode compare |
    config.hasFlow(_, sink) and compare.getAnOperand() = sink
  |
    result = compare.asExpr()
  )
}

Combining this with query established in [Step 1.6], we’ll get:

from IfStmt i
where
  i.getCond() = checkAuth() and
  i.getCond().(EqualityTestExpr).getAnOperand().(Ident).getName() = "ErrNone" and
  not i.getThen().getAStmt() instanceof ReturnStmt
select i