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 encapsulated strings (echo "$unsafe";) #3655

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

Comments

@TysonAndre
Copy link
Contributor

TysonAndre commented Jun 23, 2020

Observed: No TaintedInput is emitted, but it would be emitted if the quotes are removed
Expected: TaintedInput is emitted

$pdo->exec("select * from users where name='" . $name . "'") in https://psalm.dev/docs/security_analysis/ suggests taint detection already works for concatenation but not encapsulation

<?php

$unsafe = $_GET['unsafe'];
echo "$unsafe";
@TysonAndre
Copy link
Contributor Author

src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php analyze() seems relevant for the concat implementation.

src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php is a candidate of where to add this taint tracking for encapsulated strings?

@muglug
Copy link
Collaborator

muglug commented Jun 23, 2020

just FYI you can reproduce in psalm.dev by having <?php // --taint-analysis at the top

@weirdan
Copy link
Collaborator

weirdan commented Jun 23, 2020

just FYI you can reproduce in psalm.dev by having <?php // --taint-analysis at the top

Does it work for other switches / settings?

@muglug
Copy link
Collaborator

muglug commented Jun 23, 2020

That’s a negative (but feel free to add support for individual ones over at psalm.dev’s repo)

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

3 participants