Help: How to propagate tainting to the origin of a reference in C

[thatjiaozi]

I currently have the following query, its purpose is to find all places where an unsigned integer gets assigned to a signed variable and then, using TaintTracking check if that signed value ends up in a call to malloc:

(apologies in advance if I am doing something sub-optimal, this is the first time I use codeql)

import cpp
import semmle.code.cpp.dataflow.new.DataFlow
import semmle.code.cpp.dataflow.new.TaintTracking

predicate isBuggyPattern(AssignExpr e) {
       e.getLValue().getType().getName() = "int" and
       (e.getRValue().getType().getName().matches("unsigned%") or e.getRValue().getType().getName() = "u32")
}

predicate isAllocCall(FunctionCall fc) {
    exists(Function f | 
      f = fc.getTarget() and
      f.hasGlobalName("malloc") 
    )
}

predicate isSourceImpl(DataFlow::Node node, AssignExpr ae) {
		exists(Expr e | 
		isBuggyPattern(ae) and
        e = node.asExpr() and
        e.getParent() = ae
        )
}

predicate isSinkImpl(DataFlow::Node node, FunctionCall fc) {
        isAllocCall(fc) and
        node.asExpr() = fc.getAnArgument()
}

module SignedCastPatternConfig implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node source) {
      isSourceImpl(source, _)	
  }

  predicate isSink(DataFlow::Node node) {
      isSinkImpl(node, _)
  }
}
module SignedCastPattern = TaintTracking::Global<SignedCastPatternConfig>;


from  AssignExpr ae, File f, FunctionCall fc, DataFlow::Node src, DataFlow::Node sink
where
   f = ae.getFile() and
   fc.getFile() = f and
   isSourceImpl(src, ae) and
   isSinkImpl(sink, fc) and
   SignedCastPattern::flow(src, sink)
select ae.getLocation(), fc.getLocation()

The query works mostly fine, except for one corner case, consider the following dummy program:

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>

#define MAX_LENGTH 100
#define PREAMBLE_LENGTH 32

typedef uint32_t u32;

typedef struct {
    int len[2];
} sA;

typedef struct {
   int unused_int;
   sA nested_struct;
} sB;

u32 read_input() {
    u32 in;
    scanf("%u", &in);
    return in;
}

void allocate_buffer(sB *p) {
    int len = p->nested_struct.len[0];

    // Second usage of the tainted tracking, doesn't get detected by the query.
    char *ptr = (char *)malloc(len);
    printf("%p allocated with len %d\n", ptr, len);
    free(ptr);
}

int main(int argc, char **argv) {
    sB *instance = (sB*)malloc(sizeof(sB));
    sA *child_struct = &instance->nested_struct;
    child_struct->len[0] = read_input();

    // First usage of the tainted tracking, gets detected by the query.
    char *a = (char*)malloc(child_struct->len[0]);
    allocate_buffer(instance);
    return 0;
}

in the code above, we can see that the pointer child_struct is a reference to the nested struct in instance. child_struct is also where the assignment of u32 to an int takes place.

The first malloc call is successfully detected by the query above, however, the second call, the one inside allocate_buffer is not detected.

IIUC this is all because child_struct gets tainted with the read_input call's results, but not instance->nested_struct.len.

My question is: what is the correct way to instruct codeql to propagate the tainting so that the second malloc call gets detected?

Scouting the documentation it seems that isAdditionalTaintStep function might do the trick, but I wanted to make sure this was the right path before going down the rabbit hole.

Thanks!

[thatjiaozi]

After some experimentation and trial and error, I figured that adding the following isAdditionalTaintStep does the job:

override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
      node1 != node2 and
      exists(AssignExpr ae, VariableAccess va1, VariableAccess va2 |
      isBuggyPattern(ae) and
      ae.getAChild() = node1.asExpr() and
      va1 = node2.asExpr() and
      va1.getTarget() = va2.getTarget() and
      ae.getLValue().getAChild() = va2
      )
  }

This might be overly complicated as I am still to understand it fully, but my analysis is the following.

For the sample code I provided, when tainting gets successfully propagated, node1 gets resolved to the read_input() call in child_struct->len[0] = read_input(); while node2 gets resolved to the variable access of len in int len = p->nested_struct.len[0];

Now, my interpretation of why these values, and please take in mind that it might be utterly wrong as I don't know what is going on, is that codeql at some point asks itself if the access of the len variable should be tainted by the function call's value.

Thus, my isAdditionalTaintStep predicate holds true if the variable access to be tainted is part of an assign expression where the function call read_input also takes place.

In other words: take an additional taint step if the variable access has been involved with the results of the function call

I'd still like to know more on the why of this behaviour, and if my assessment is correct tho :)

[jketema]

Hi @thatjiaozi,

I've asked internally how to best deal with this. It might be a while before I get an answer, because people are still on holiday after New Year's.

[jketema]

Hi @thatjiaozi,

What you want is in fact not supported. isAdditionalTaintStep is not taken into account when the access path is non-empty (i.e., when a field of a class/struct is what is tainted). We are aware of this, and would like to fix this, but we currently have no timeline for this.