github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.32k stars 1.47k forks source link

I have setup a project to test the capability of CodeQL,to test taint tracking ablitity #11752

Open hatface opened 1 year ago

hatface commented 1 year ago

here is my project

https://github.com/hatface/codeql_cap_evalution

here is the test result

https://github.com/hatface/codeql_cap_evalution/blob/main/CodeQL_Capacibility_Test_Result.md

here is the CodeQL Query

https://github.com/hatface/codeql_cap_evalution/blob/main/taint_cap_test.ql

and the case5 is failed

QUESTION: how to fix the situation using isAdditionalTaintStep

jketema commented 1 year ago

Hi @hatface. Thanks for your question. As we have been discussing on https://github.com/github/codeql/issues/11734, CodeQL might not give you back all the paths. When I modify your main function to be just

     source(sourceInt, sourceStr);
     sceneTest05(sourceInt, sourceStr);

the relevant path shows up for me. You really want to disentangle your test cases. So instead of having a single main function with overlapping test cases create 6 separate ones that look something like:

void test0() {
    ...
     source(sourceInt, sourceStr);
     sink(sourceInt, sourceStr);
}

void test1() {
    ...
     source(sourceInt, sourceStr);
     sceneTest01(sourceInt, sourceStr);
}

...

void test5() {
    ...
     source(sourceInt, sourceStr);
     sceneTest05(sourceInt, sourceStr);
}

int main(int argc, char **argv) {
  test0();
  test1();
  ...
  test5();
  return 0;
}
hatface commented 1 year ago

@jketema thx a lot, but I followed your advice, the test case 5 still fail, my project, my QL, my test result updated

jketema commented 1 year ago

Are you sure the path is actually missing? With the latest version of your project I'm seeing the following, which I your 5th test case.

Screenshot 2022-12-20 at 10 36 50
hatface commented 1 year ago

emm, it seems that we encounter a problem,would you pls use the lastest version of my program,and query with the lastest ql。all resource in here https://github.com/hatface/codeql_cap_evalution。 I can't find the testcase 5.

ps:codeql-repo and codeql-binary-cli is the lastest

jketema commented 1 year ago

My result above is with the latest versions.

jketema commented 1 year ago

Which operating system are you on and which compiler do you use?

hatface commented 1 year ago

compile on ubuntu compiler:gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)

run result on windows with vscode-codeql and codeql on windows

jketema commented 1 year ago

compile on ubuntu compiler:gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)

I haven't checked on Windows, but I see your issue on Linux. This issue is a combination of the dataflow library you're using and the way shared pointers are implemented by gcc. The solution is to switch to our IR-based dataflow library. To do so, replace

import semmle.code.cpp.dataflow.TaintTracking

by

import semmle.code.cpp.ir.dataflow.TaintTracking
hatface commented 1 year ago

can you share the testcases,which is used by codeQL team?

jketema commented 1 year ago

can you share the testcases,which is used by codeQL team?

I'm not sure what you mean. I just tested with the project you shared.

hatface commented 1 year ago

I mean share CodeQL team's testcases,which your team use to test the ability of CodeQL,or benchmark code,may be

jketema commented 1 year ago

All the test cases are located in https://github.com/github/codeql

hatface commented 1 year ago

compile on ubuntu compiler:gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)

I haven't checked on Windows, but I see your issue on Linux. This issue is a combination of the dataflow library you're using and the way shared pointers are implemented by gcc. The solution is to switch to our IR-based dataflow library. To do so, replace

import semmle.code.cpp.dataflow.TaintTracking

by

import semmle.code.cpp.ir.dataflow.TaintTracking

by replace this import statement,testcase 5 is passed,but testcase 1, testcase 2, testcase 3, testcase 4 is failed

hatface commented 1 year ago

I haven't checked on Windows

foreign programer used to work on ubuntu,or mac,save a lot of time。 but we programer of china used to work on windows,haha。

jketema commented 1 year ago

by replace this import statement,testcase 5 is passed,but testcase 1, testcase 2, testcase 3, testcase 4 is failed

Try:

from DataFlow::PathNode sink, DataFlow::PathNode source, MyTaintTrackingConfiguration config
where config.hasFlowPath(source, sink)
select sink.getNode().asExpr() ,source, sink,"the taint flow into sink from $@.", source.getNode().asExpr(), "source"

instead of

from DataFlow::PathNode sink, DataFlow::PathNode source, MyTaintTrackingConfiguration config
where config.hasFlowPath(source, sink)
select sink.getNode() ,source, sink,"the taint flow into sink from $@.", source.getNode(), "source"
hatface commented 1 year ago

by replace this import statement,testcase 5 is passed,but testcase 1, testcase 2, testcase 3, testcase 4 is failed

Try:

from DataFlow::PathNode sink, DataFlow::PathNode source, MyTaintTrackingConfiguration config
where config.hasFlowPath(source, sink)
select sink.getNode().asExpr() ,source, sink,"the taint flow into sink from $@.", source.getNode().asExpr(), "source"

instead of

from DataFlow::PathNode sink, DataFlow::PathNode source, MyTaintTrackingConfiguration config
where config.hasFlowPath(source, sink)
select sink.getNode() ,source, sink,"the taint flow into sink from $@.", source.getNode(), "source"

by this,testcase 5,testcase 3,testcase 2,testcase 1 is passed,testcase4 failed

jketema commented 1 year ago

Hi @hatface, I've checked internally and failure of your 4th test case seems to be due to a bug on our side. The bug cannot be worked around with isAdditionalTaintStep. I have filed an internal issue to track this problem.

ryao commented 1 year ago

I haven't checked on Windows

foreign programer used to work on ubuntu,or mac,save a lot of time。 but we programer of china used to work on windows,haha。

This might be useful:

https://ubuntu.com/wsl

It lets you run Ubuntu software on Windows, provided that you have Windows 10 version 1607 or later, and your Windows installation is 64-bit.

hatface commented 1 year ago

@jketema hello jketema,I have new tests,and testcase 14,testcase 17,testcase 18,testcase 21,testcase 22,testcase 23,testcase 24,testcase 30,testcase 33 is failed,the same question how to fix this ,by isAdditionalTaintStep the test result is shown here

https://github.com/hatface/codeql_cap_evalution/blob/main/CodeQL_Capacibility_Test_Result.md

and my project is updated

jketema commented 1 year ago

Hi @hatface,

It's impossible for us to look in detail at your test cases without explanation from your side of (a) what a test case is aimed to test, (b) how you have tried to extend isAdditionalTaintStep yourself to handle a particular test case and why that didn't work, and (c) with which of the data flow libraries the test is failing (semmle.code.cpp.dataflow.TaintTracking, semmle.code.cpp.ir.dataflow.TaintTracking, or both). So, please provide that information.

hatface commented 1 year ago

sorry,I will explain my testcases, the following tables shows the the test case aimed to test.

index aimed to test test function explain result other words
14 ternary operator sceneTest0014 test if when ternary operator in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
17 function pointer sceneTest0017 test if when function pointor in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
18 function pointer of std::function sceneTest0018 test if when function pointor of std::function in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
21 virtual function sceneTest0021 test if when virtual function in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
22 virtual function1 sceneTest0022 test if when virtual function in the path is the taint tracking works well as a positive of 21 fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
23 typedefed function pointer sceneTest0023 test if when typedefed function point in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
24 pre build-in template function sceneTest0024 test if when pre build-in template function in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
30 static member field sceneTest0030 test if when static member field in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
31 decontruction function using with smart pointer sceneTest0033 test if whendecontruction function using with smart pointer in the path is the taint tracking works well fail semmle.code.cpp.dataflow.TaintTracking and semmle.code.cpp.ir.dataflow.TaintTracking both fail
jketema commented 1 year ago

I will explain my testcases, the following tables shows the the test case aimed to test.

Thanks. So in each case how did you try to extend isAdditionalTaintStep to try to fix the problem?

hatface commented 1 year ago

I did'nt extend isAdditionalTaintStep by myself, but some of the test case I have tried before, for example testcase 30, I can't find an elegent way to extend isAdditionalTaintStep in the way of programing language, but only extend isAdditionalTaintStep in the way of business, so, I test the programing language feature, and marked all the failed testcase, as a first step, lying them in the issue, and looking for help. I do want to fix them case by case.

jketema commented 1 year ago

I'm happy to help, but do expect some kind of effort from your side. In this case you should first really try to modify isAdditionalTaint step yourself. We cannot do all your work for you.