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++] Taint analysis does not appear to handle aliasing #18151

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

[C++] Taint analysis does not appear to handle aliasing #18151

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

Comments

@JustusAdam
Copy link

Taint analysis appears to loose taint whenever assignments to pointer aliases are involved.

This first example is very simple but the flow from source to sink is not found. It appears to me as though there is no alias analysis being performed if this simple case doesn’t work?

__attribute__((noinline)) int source()
{
    return 2;
}

__attribute__((noinline)) int target(int source)
{
    return source;
}

void test_values() {
    int *a = new int[1];
    int *b = a;
    a[0] = source();
    target(b[0]);
    target(a[0]);
}

This is the output I get from this example. Only the flow via a is detected.

|     source     | source_line |        sink        | sink_line |
+----------------+-------------+--------------------+-----------+
| call to source |          14 | access to array    |        16 |

I had wondered if maybe an alias analysis is only performed to resolve dynamic dispatch, but in this simple case it has the same problem. Only the first flow is found, not the second one

int a_function()
{
    return source();
}

int b_function()
{
    return 0;
}

int test_dispatch()
{
    int (*fptr)();

    fptr = a_function;

    target(fptr());

    int (*fptr2)();
    int (**fptr2_ptr)() = &fptr2;
    fptr2 = b_function;
    *fptr2_ptr = a_function;

    target(fptr2());

    return 0;
}

This is the output. (The line numbers are off by 10, because I’ve omitted defining source and target again).

|     source     | source_line |        sink        | sink_line |
+----------------+-------------+--------------------+-----------+
| call to source |          13 | call to expression |        27 |

The query I run in all of these cases is the following

import cpp
import semmle.code.cpp.dataflow.new.TaintTracking

module SourceSinkCallConfig implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node source) {
    source.asExpr().(Call).getTarget().getName() = "source"
  }

  predicate isSink(DataFlow::Node sink) {
    exists(Call call |
      call.getTarget().getName() = "target" and
      call.getArgument(0) = sink.asExpr()
    )
  }
}

module SourceSinkCallTaint = TaintTracking::Global<SourceSinkCallConfig>;

from DataFlow::Node source, DataFlow::Node sink, int source_line, int sink_line
where
  SourceSinkCallTaint::flow(source, sink) and
  source_line = source.getLocation().getStartLine() and
  sink_line = sink.getLocation().getStartLine()
select source, source_line, sink, sink_line

CodeQL version: 2.19.3

@redsun82
Copy link
Contributor

redsun82 commented Nov 28, 2024

👋 @JustusAdam

Indeed we currently don't yet do alias analysis in taint flow, the problem is actually related to what you raised in #18101. This is something we're working on: at some point we had implemented it, but we had to roll back because of performance issues. However, we're working to get those resolved and make alias analysis work with data flow and taint tracking.

@JustusAdam
Copy link
Author

Hmm, that is unfortunate. Is this only the case in C++ or also in other supported languages?

@redsun82
Copy link
Contributor

Sorry, I don't currently know what's the status of this in other languages. Any specific language you are interested in?

@JustusAdam
Copy link
Author

Yes, Java and JavaScript in particular.

@redsun82
Copy link
Contributor

redsun82 commented Dec 2, 2024

As far as I know, aliasing in Java and Javascript is restricted to (implicit) references and is covered (no pointers to worry about).

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
@redsun82 @JustusAdam and others