C++: Taint tracking through std::shared_ptr

Hello,

the following sample program illustrates the issue:

int taint_src(void) {
  return 42;
}

void getNumber(std::shared_ptr<int> ptr) {
  *ptr = taint_src();
}

int main(void) {
  std::shared_ptr<int> ptr_main(new int);

  getNumber(ptr_main);

  return *ptr_main;
}

The idea is to track the taint flow from the call to taint_src to the return value of main.

I'm having problems modelling getNumber to specify that ptr_main should be tainted after the call to getNumber returns. None of the predicates in DataFlowFunction or TaintFunction seem to be able to model this, as the argument ptr_main is passed by value. Is there anyway to model the taint flow for such cases?

(If anyone is trying to model this, do note that std::shared_ptr::operator* is const qualified, and hence Issue 5116 will also crop up until it is fixed.)

I passed the question on to our C/C++ analysis experts in @github/codeql-c-analysis. They agree that the taint analysis is being confused by the use of std::shared_pointer rather than a reference or primitive C pointer for the output parameter, and are considering what the best way to un-confuse it would be. Stay tuned.

@creative16

Hi @creative16,

Thanks for bringing this to our attention. This is indeed a problem that's not quite easy to solve currently. If the shared_ptr was passed by reference it should be possible to use Node.asDefiningArgument() to add a custom taint rule to the configuration.

We're currently working on increased model coverage of the standard library, and shared_ptr is coming up on our list very soon. Hopefully, this will solve the problem you're hitting!

p.s.: #5116 has now been fixed in #5196.

@MathiasVP

Hello @MathiasVP,

thanks for the reply. Looking forward to the improvements! And yep, I noticed the fix for #5116 a few days ago. Thanks for fixing it!

C++: Add test from issue github#5190.

@creative16

Hi @creative16,

We've improved our support for smart pointers and on the current main branch I now see flow in your example program with this query:

/**
 * @kind path-problem
 */
import cpp
import semmle.code.cpp.dataflow.DataFlow
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph

class Config extends TaintTracking::Configuration {
  Config() { this = "Config" }

  override predicate isSource(DataFlow::Node source) {
    source.asExpr() = any(Call c | c.getTarget().hasName("taint_src"))
  }

  override predicate isSink(DataFlow::Node sink) {
    exists(ReturnStmt ret | sink.asExpr() = ret.getExpr())
  }
}

from DataFlow::PathNode source, DataFlow::PathNode sink, Config c
where c.hasFlowPath(source, sink)
select sink.getNode(), source, sink, ""

If you find other issues be sure to let us know!

@MathiasVP

Hello @MathiasVP,

thanks so much for the fixes. Appreciate it. I'll be sure to let you guys know if I hit other issues. Thanks again!

The improvement should be live on LGTM and Code Scanning. I'll close this ticket, expecting that things are working.

Navigation Menu

Repository navigation

C++: Taint tracking through std::shared_ptr #5190

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

C++: Taint tracking through std::shared_ptr #5190

Description

Activity

hmakholm commented on Feb 17, 2021

MathiasVP commented on Feb 23, 2021

creative16 commented on Feb 23, 2021

MathiasVP commented on Apr 15, 2021

creative16 commented on Apr 20, 2021

jbj commented on May 3, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions