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

Taint detection does not work for global functions in a namespace due to id not including namespace #3654

Closed
TysonAndre opened this issue Jun 23, 2020 · 0 comments
Labels

Comments

@TysonAndre
Copy link
Contributor

TysonAndre commented Jun 23, 2020

I see that the graph that was built has an edge with a from_id of identity#1, but elsewhere, there's an edge to ns\identity#1 with a prefix of ns\

It also looks like there might be potential problems normalizing the case of namespace (should be consistently lowercased or uppercased to properly uniquely track?), but the lack of the prefix seems to be the main problem.

Using https://github.com/TysonAndre/psalm/compare/taint-debug?targetBranch=master to print debugging info about sources/edges

Expected: Should emit TaintedInput
Observed: Does not

<?php

namespace ns {

function identity($s) {
    return $s;
}

echo namespace\identity($_GET['userinput']);

}
/*

Sources:
$_GET:src/namespaced_taint.php:89
Edges:
From identity#1  -- this is the $from_id
-> ns\identity
From $_GET:src/namespaced_taint.php:89
-> $_GET['userinput']-src/namespaced_taint.php:89-93
From call to NS\identity-src/namespaced_taint.php:89-106
-> ns\identity#1   -- this is the $to_id
From $_GET['userinput']-src/namespaced_taint.php:89-93
-> call to NS\identity-src/namespaced_taint.php:89-106
From call to echo-src/namespaced_taint.php:70-107
-> echo#1-src/namespaced_taint.php:65
From ns\identity
-> call to echo-src/namespaced_taint.php:70-107
Sinks:
echo#1-src/namespaced_taint.php:65
 */
@muglug muglug added the bug label Jun 23, 2020
@muglug muglug closed this as completed in 96d05ab Jun 23, 2020
TysonAndre pushed a commit to TysonAndre/psalm that referenced this issue Jun 24, 2020
TysonAndre pushed a commit to TysonAndre/psalm that referenced this issue Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants