Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hatface opened this issue Dec 20, 2022 · 24 comments
Labels
C++ question Further information is requested

Comments

@hatface
Copy link

hatface commented Dec 20, 2022

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

@hatface hatface added the question Further information is requested label Dec 20, 2022
@hatface hatface changed the title I have setup a project to test the capability of CodeQL,to test I have setup a project to test the capability of CodeQL,to test taint tracking ablitity Dec 20, 2022
@jketema
Copy link
Contributor

jketema commented Dec 20, 2022

Hi @hatface. Thanks for your question. As we have been discussing on #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
Copy link
Author

hatface commented Dec 20, 2022

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

@jketema
Copy link
Contributor

jketema commented Dec 20, 2022

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
Copy link
Author

hatface commented Dec 20, 2022

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
Copy link
Contributor

jketema commented Dec 20, 2022

My result above is with the latest versions.

@jketema
Copy link
Contributor

jketema commented Dec 20, 2022

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

@hatface
Copy link
Author

hatface commented Dec 20, 2022

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
Copy link
Contributor

jketema commented Dec 20, 2022

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

@jketema jketema added the C++ label Dec 20, 2022
@hatface
Copy link
Author

hatface commented Dec 20, 2022

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

@jketema
Copy link
Contributor

jketema commented Dec 20, 2022

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
Copy link
Author

hatface commented Dec 20, 2022

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

@jketema
Copy link
Contributor

jketema commented Dec 20, 2022

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

@hatface
Copy link
Author

hatface commented Dec 20, 2022

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
Copy link
Author

hatface commented Dec 20, 2022

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
Copy link
Contributor

jketema commented Dec 20, 2022

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
Copy link
Author

hatface commented Dec 20, 2022

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
Copy link
Contributor

jketema commented Dec 21, 2022

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
Copy link

ryao commented Dec 21, 2022

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
Copy link
Author

hatface commented Dec 29, 2022

@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
Copy link
Contributor

jketema commented Dec 29, 2022

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
Copy link
Author

hatface commented Dec 30, 2022

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
Copy link
Contributor

jketema commented Dec 30, 2022

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
Copy link
Author

hatface commented Jan 2, 2023

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
Copy link
Contributor

jketema commented Jan 2, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants