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.53k stars 1.5k forks source link

General issue #4843

Open Conanjun opened 3 years ago

Conanjun commented 3 years ago

Dataflow Bug: codeql javascript Dataflow break with normal parameter (like function a({data1,data2,data3})) pass

Hello Cool Codeql Guys, i have found a bug when i use dataflow to analyse my javascript with taintpath.

my taintpath query code is normal code just like this: from RiskTaint cfg, PathNode source, PathNode sink where cfg.hasFlowPath(source, sink) select sink.getNode(), source, sink, "risk funcation call with user-controlled input." however , when i try to analyse the test code is like this: function func_a(param_a){ local_a=param_a.aaa ... ... const { var_a } = await var_b.method_c({local_a,var_c}) // line a: not correctly get the taint path //const { var_a } = await var_b.method_c(local_a,var_c) // line b: correctly get the taint path ... ... } when i use line a to build the query database and analyse taint path , i can't get the taint path from param_a to local_a which was an argument of var_b.method_c,then i try to change line a to line b and it works,so i think it may be the dataflow bug in codeql javascript.

Thks a lot for dealing with my issue,i think codeql is a really really great cool artwork !!!

asgerf commented 3 years ago

On line a, the argument is an object, and local_a is just one of its properties. If you tried to use the argument itself as a sink you won't get flow there.

In that case you can use getOptionArgument to find the argument:

override predicate isSink(DataFlow::Node node) {
  node = get_method_c_call().getOptionArgument(0, "local_a") // for line a
}

On line b, local_a is passed as argument 0. In this case, use getArgument(0):

override predicate isSink(DataFlow::Node node) {
  node = get_method_c_call().getArgument(0) // for line b
}

If this did not answer your question, please provide more of your CodeQL code.

Conanjun commented 3 years ago

On line a, the argument is an object, and local_a is just one of its properties. If you tried to use the argument itself as a sink you won't get flow there.

In that case you can use getOptionArgument to find the argument:

override predicate isSink(DataFlow::Node node) {
  node = get_method_c_call().getOptionArgument(0, "local_a") // for line a
}

On line b, local_a is passed as argument 0. In this case, use getArgument(0):

override predicate isSink(DataFlow::Node node) {
  node = get_method_c_call().getArgument(0) // for line b
}

If this did not answer your question, please provide more of your CodeQL code.

unfortunately,the sink is not here because it is in the middle part like ctx->param_a -> linea {local_a,var_c} -> other argument of function in var_b.method_c such as eval() , i don't know how to solve it in this scene hahaha ... ...

Conanjun commented 3 years ago
getOptionArgument

In the same time , i change my query code to use getOptionArgument , it doesn't work,i have checked the code and make it can get the flow correctly when i try to use line-b pattern== image

image

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 14 days with no activity. Comment or remove the stale label in order to avoid having this issue closed in 7 days.