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

Security ("runTaintAnalysis") results not shown #25

Closed
Danysan1 opened this issue Dec 17, 2020 · 3 comments
Closed

Security ("runTaintAnalysis") results not shown #25

Danysan1 opened this issue Dec 17, 2020 · 3 comments

Comments

@Danysan1
Copy link

Danysan1 commented Dec 17, 2020

In psalm.xml I have enabled the runTaintAnalysis parameter to enable SAST:

<?xml version="1.0"?>
<psalm
    errorLevel="3"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    runTaintAnalysis="true"
>
    <projectFiles>
        ...
    </projectFiles>
</psalm>

If I run psalm from CLI I get various taint alerts, for example:

...

ERROR: TaintedHtml - anteprima.php:324:22 - Detected tainted HTML (see https://psalm.dev/245)
  $_GET
    <no known location>

  $_GET['com_id'] - anteprima.php:13:40
    $com_id = isset($_GET["com_id"]) ? $_GET["com_id"] : -1;

  $com_id - anteprima.php:13:5
    $com_id = isset($_GET["com_id"]) ? $_GET["com_id"] : -1;

  call to echo - anteprima.php:324:22
                let idComune = <?= $com_id; ?>,

...

If I visit this file with VSCode and the psalm plugin enabled, however, I see other alerts from Psalm but not the taint/security ones.

@Danysan1
Copy link
Author

The root of this problem is in Psalm itself as apparently its creators don't intend to implement it into the language server (see vimeo/psalm#4869 ), closing this.

@muglug
Copy link
Member

muglug commented Dec 21, 2020

Yup, sorry about that – on Vimeo's codebase, for example, taint analysis takes a few minutes to run. That's not sustainable in an IDE!

@Danysan1
Copy link
Author

Danysan1 commented Jan 10, 2021

Taint analysis could be configurated separately for the language server (for example with an argument like languageServerTaintAnalysis or forceTaintAnalysis). This way taint analysis could be enabled for language server only if the codebase is small enough while enabled separately for CLI. If I proposed a pull request with this feature would you merge it?

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

No branches or pull requests

2 participants