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

[C++] Fails to detect control flow influence of nested “if” #18099

Open
JustusAdam opened this issue Nov 25, 2024 · 5 comments
Open

[C++] Fails to detect control flow influence of nested “if” #18099

JustusAdam opened this issue Nov 25, 2024 · 5 comments
Labels
question Further information is requested

Comments

@JustusAdam
Copy link

The controls predicate in GuardCondition fails to detect a control flow influence from a nested if. In the following example the influence from condition to call() is only revealed in the first if, but not in the second.

#include <exception>

void call()
{
}

void my_fn(bool outer, bool condition)
{

    if (condition) // detected
    {
        throw std::exception();
    }

    if (outer)
    {
        if (condition) // not detected
        {
            throw std::exception();
        }
    }

    call();
}

Query I tried:

import cpp
import semmle.code.cpp.controlflow.IRGuards

from Variable v, VariableAccess va, GuardCondition cond, Call c, int line
where
  c.getTarget().getName() = "call" and
  va.getTarget() = v and
  v.getName() = "condition" and
  cond.getAChild*() = va and
  cond.controls(c.getBasicBlock(), _) and
  line = va.getLocation().getStartLine()
select v, va, cond, c, line

Output I received:

|     v     |    va     |   cond    |      c       | line |
+-----------+-----------+-----------+--------------+------+
| condition | condition | condition | call to call |   10 |

I expected to also see an influence from line 17, but none is being found.

CodeQL version: 2.19.3

@JustusAdam JustusAdam added the question Further information is requested label Nov 25, 2024
@redsun82
Copy link
Contributor

hi @JustusAdam

I think you may be misunderstanding what GuardCondition.support means. From its doc:

The predicate holds if all paths to controlled go via the testIsTrue edge of the control-flow graph.
(emphasis on "all" is mine)

This is not the case for the nested condition, as there is a path going through outer == false that avoids the nested if altogether. Similarly, neither outer == false nor outer == true control the basic block of call(), as they both have a path going to call() through them (so neither have exclusivity on the paths to call()). As a consequence, the only GuardCondition controlling the basic block of call() is indeed the one you found, via the false value.

Incidentally, from the snippet you posted and the presence of the line in your query, I'm assuming you are running codeql in the CLI. I think that for playing around with CodeQL the best might be to use VSCode with the CodeQL extension. Apart from powerful debugging tools like quick evaluation, when running queries the results are clickable allowing to highlight the result in the extracted code.

@redsun82
Copy link
Contributor

Incidentally, from the snippet you posted and the presence of the line in your query, I'm assuming you are running codeql in the CLI. I think that for playing around with CodeQL the best might be to use VSCode with the CodeQL extension. Apart from powerful debugging tools like quick evaluation, when running queries the results are clickable allowing to highlight the result in the extracted code.

Obviously disregard this comment if you just ran on the CLI to paste the result here 😅

@JustusAdam
Copy link
Author

Hi, thanks for the extensive answer.

I had indeed misunderstood the predicate. So my question is wether such a "weaker" predicate exists that checks whether a given condition can influence a basic block?

Thanks for the side note about the extension, I might try that though in general when it comes to program analysis tools I like to reduce the indirections to ensure I understand what the tool does and hence prefer a CLI 😅 . But I might give it a try.

@redsun82
Copy link
Contributor

I don't think there is a weaker version like that. The main use case of GuardCondition is to provide barriers for taint flow: if data might be tainted, but before feeding it to a sink you perform a check on the data that controls at runtime whether the data is clean, then we want to not have taint flow under the guard. For that reason, we really need to know whether a given arm of a guard completely covers a sink.

@rvermeulen
Copy link
Contributor

Hi @JustusAdam,

Can you clarify what you mean with "condition can influence"?
Do you want to find all the successors of that condition to understand how a value in the condition influences the control flow?

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

No branches or pull requests

3 participants