cisco-open / llvm-crash-analyzer

llvm crash analysis
Apache License 2.0
40 stars 17 forks source link

[WIP][TA] Analyze out-of-BT func with tainted parameters #47

Open niktesic opened 1 year ago

niktesic commented 1 year ago

Summary

This patch introduces support for analysis of functions, which call site is not in the backtrace, if the parameter is tainted. For now, only parameters forwarded through registers are supported.

This partially resolves https://github.com/cisco-open/llvm-crash-analyzer/issues/36

Description

To be able to track the value of function parameters, this patch introduces forward TaintAnalysis, which propagates the Taint from the function entry point to the point where the parameter value is set. If the path of the Taint is not terminated in the call (it is not set to constant value like in the tainted-param-inside-blame.test ), we should run the backward TaintAnalysis to try to propagate the Taint from the point where the parameter is set to a value, to the point where that value is set to constant ( tainted-params-outside-blame.test ).

Content of commits

0001-Target-Helpers-for-param-forwarding-registers.patch

0002-TaintDFG-Upgrade-TaintDFG-infrastructure.patch

0003-TA-Detect-tainted-parameters.patch

0004-TA-Introduce-forward-Taint-Analysis.patch

0005-TA-Analysis-of-already-decompiled-functions.patch

0006-New-tests-for-tainted-params-analysis.patch

Example

// clang -g -O0 m.c -o m
void crash(int val, int* adr){
    *adr = val; // crash - line 3
}

void blame(int** ptr, int* adr){
    *ptr = adr; // wrong blame - line 7
}

void fun(int** ptr, int* adr)
{
   adr = 4; // correct blame - line 12
   blame(ptr,adr);
}

int main(){
  int *adr = 3; // wrong blame - line 17
  int *p = 0; // wrong blame - line 18
  fun(&p,adr);
  crash(1, p);
  return 0;
}

Run crash analyzer: llvm-crash-analyzer --print-potential-crash-cause-loc ./m --core-file core.m Output:

core-file processed.

Decompiling...
Decompiling crash
Decompiling main
Decompiled.

Analyzing...
Decompiling fun
Decompiling blame

Blame Function is fun
From File m.c:12:8

TaintDFG: mir-dfg

niktesic commented 1 year ago

Thank you for the extensive work, I didn't see much functional issues in the code, but there are couple efficiency issues that I commented on and also the style is a bit hard to understand. for example, runOnBlameMF has grown too big and the added complexity of two flags runFwAnalysis and runBwAnalysis hasn't made it easier to understand. Nevertheless, I'm ok with this code checked in, so I'm approving it, but not merging it. Waiting for other team members to comment

Thanks for your comments, Ali! I will add a new commit with suggested updates. Feel free to add new suggestions, if you notice some.

niktesic commented 1 year ago

New commits address the reviewers comments and introduce support for parameters on the stack. This PR is functionally complete in terms of analysis of functions out of the backtrace, where parameters are tainted. New commits discard previous approval, so a new one is needed. Thanks!

Content of commits in new changes

0007 Addressing comments 1

0008 [TA] Support parameters on the stack