[Sonar Source] Adding support of providing Vulnerability definition for SAST tools

[preetkaran20]

Is your feature request related to a problem? Please describe. Similar to the Vulnerability definition we provide for DAST, we need to add the support for SAST tools too. Along with this, please look at fixing DAST Vulnerability definition also to include minimal information

Describe the solution you'd like Request for OWASP VulnerableApp.pdf

Solution: I think we should add a parser which reads the java file (This will be a compile time generation of vuln definition and then pushed to github) and provides the estimated lines of code snippets which are having annotations of VulnerableApp. Also we might need to add some levels of Secure implementations such that false positives can be better judged.

[preetkaran20]

As this is a request from Sonar Source team so we need to work on this at the earliest.

[preetkaran20]

@agigleux Tagging for reference.

[agigleux]

Given the quantity of vulnerabilities demonstrated (ie. less than 100), I'm not sure it's required to build an automated solution to generated the issues that should be detected automatically by a SAST engine. I think a first acceptable solution could be to take a couple of minutes and manually create the "ExpectedIssues.csv".

I will create a PR with such a file so we can exchange about its content in case I did a mistake, or missed some vulnerabilities.

[preetkaran20]

Given the quantity of vulnerabilities demonstrated (ie. less than 100), I'm not sure it's required to build an automated solution to generated the issues that should be detected automatically by a SAST engine. I think a first acceptable solution could be to take a couple of minutes and manually create the "ExpectedIssues.csv".

I will crreate a PR with such a file so we can exchange about its content in case I did a mistake, or missed some vulnerabilities.

Yeah makes sense. In the mean time, i am looking at automated way for doing this for long term solution.

[fdmcneill2019]

Hi, is this issue still available?

[preetkaran20]

Hi @fdmcneill2019 ,

Yes, we have added vulnerabilities manually to a csv but we are looking for the automated way to populate it.

thanks, Karan

[fdmcneill2019]

Hi @fdmcneill2019 ,

Yes, we have added vulnerabilities manually to a csv but we are looking for the automated way to populate it.

thanks, Karan

Could you explain exactly what is meant by "non expected issues"? Does this term refer to unexpected issues detected by a SAST tool as against a OWASP parser? Also, I read the attached pdf file regarding the feature request but I'm confused as to what Alexandre meant by "FN" and "TN" in his formulas for finding the True Positive Rate and False Positive Rate. If you wouldn't mind translating them for me, that'd be great.

Thanks

[preetkaran20]

Hi @fdmcneill2019 , Yes, we have added vulnerabilities manually to a csv but we are looking for the automated way to populate it. thanks, Karan

Could you explain exactly what is meant by "non expected issues"? Does this term refer to unexpected issues detected by a SAST tool as against a OWASP parser? Also, I read the attached pdf file regarding the feature request but I'm confused as to what Alexandre meant by "FN" and "TN" in his formulas for finding the True Positive Rate and False Positive Rate. If you wouldn't mind translating them for me, that'd be great.

Thanks

Hi @fdmcneill2019 ,

Could you explain exactly what is meant by "non expected issues"?

So not expected issues are something which are not present in vulnerableapp but are reported by Sast scanner.

Also, I read the attached pdf file regarding the feature request but I'm confused as to what Alexandre meant by "FN" and "TN" in his formulas for finding the True Positive Rate and False Positive Rate.

Actually those details of True positive rate or false positive rate will be evaluated by Sast scanners. so we need not to worry about those details. This issue is where we need to automated the generate of [csv] (https://github.com/SasanLabs/VulnerableApp/blob/master/scanner/sast/expectedIssues.csv)

thanks, Karan

[fdmcneill2019]

@preetkaran20 Thank you very much for the responses so far. I just have one more basic one: If I think I have a possible solution (idea or incomplete source code), what the most ideal way I should go about sharing it? Is there a dedicated forum for discussing problem-solving strategies with other developers? Specifically, you and whoever else is working on this issue.

Thanks again.

[preetkaran20]

Hi @fdmcneill2019,

The process is that you can add simple discussions details in the issue only so that it can be backtracked also you can write a wiki in case of design and possible solutions discussions and link that wiki to this issue.

thanks, Karan

[preetkaran20]

Hi @agigleux ,

I have one idea on automating the SAST CSV generation by creating a wrapper method which will execute the vulnerable code and also log the details which we need. Something similar to below mentioned code snippet:

    interface SupplierWithException<T,E extends Exception> {
        T get() throws E;
    }

    static class Wrapper {
        <T, E extends Exception> T wrap(SupplierWithException<T,E> supplier) throws E {
            System.out.println(Thread.currentThread().getStackTrace()[1].getLineNumber());
            System.out.println(Thread.currentThread().getStackTrace()[1].getClass().getMethods())
            return supplier.get();
        }
    }

However i am facing one issue because of this multiline thing i.e. :

Files.copy(
file.getInputStream(),
root.resolve(fileName), 
StandardCopyOption.REPLACE_EXISTING);

I think SAST tool will report the exact line which can cause the vulnerability but the above approach will not be able to provide the exact line number. So i thought that we can have a buffer line +/-3.

What is your take on this ? @fdmcneill2019 In case you have something else in mind, please suggest.

thanks, Karam

[agigleux]

Hello,

I think it's important to get the exact line number where the vulnerability can be exploited to measure how precise is the SAST tool.

Overall, I wonder if this is really required to spend time trying to automatically generate such CSV file. I think it could be OK to rely on a manual process saying that each time a new vulnerability example is added to this VulnerableApp, it's mandatory to document where is the expected issue for SAST tools. Because if you think about it, it's not that often that vulnerability examples are added to VulnerableApp.

[preetkaran20]

Hi @agigleux,

The actual issue is that we use tools that format the files (like spotless apply) and in case there is a small modification done in the file, it requires changing the line numbers in the CSV. It will become more and more difficult to manage the CSV with the addition of vulnerabilities in the project and also adding line numbers is cumbersome for the developers of vulnerability.

One question, how will SAST report line number for the following use case:

function vulnerableFunction(input){
return process.exec(
'cmd ',
input
);
}

In this case, say the input is user-controlled and it causes the command to be executed on the server. Which line does SAST report?

thanks, Karan

[agigleux]

For SonarQube or SonarCloud, the line number will be at the line containing process.exec( (ie: at the location of the "sink").

[preetkaran20]

and in case it is

process.
exec

or

process
.exec

would it still be the first line?

[agigleux]

Even if I doubt that such code will exist in real life, I would expect the issue to be raised on the exec's line.

[preetkaran20]

Hi @agigleux,

ok, so I am thinking of the following plan:

1. I will try to run Sonarqube on VulnerableApp
2. Analyse the report
3. Maybe if I find any other important flag to consider, will include it in the CSV.

I also think that pinpointing the exact line where the vulnerability is, seems a little challenging, and requires a lot more understanding of the sonar reports. It can be the next step.

In the first version, we can give some approximation i.e. say the statement is spread out in 3 lines then we will give the line number and +3 as delta, and as we know that, the file and method will have that only vulnerability and if it matches it seems a reasonable work.

What is your take on this?

thanks, Karan

[agigleux]

I think you should wait for https://jira.sonarsource.com/browse/MMF-1700 to be implemented before trying to scan VulnerableApp with SonarQube/SonarCloud. VulnerableApp is relying a lot on lambda expressions and our security engine doesn't support such syntax yet. The work to support it will start in the coming days and should be part of SonarQube 9.0 (end of June 2021).

About your approach, I'm fine with that. That's a good first step and maybe someone else with improve the location accuracy later.

[preetkaran20]

ok, Thanks for the update.

[preetkaran20]

Hi @agigleux ,

I have again revisited this and thought about the problem, I am thinking, how about if we provide you a comment indicator on the line where Vulnerability exists. Would that suffice the requirement? This solution will work for PHP, JSP, ASP and other tech stacks.

thanks, Karan

[agigleux]

Hello @preetkaran20,

I think this could answer to the need to have the list of vulnerabilities documented. The best would be to add a script to automatically generate the CSV file from the comment indicators.

Alex