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

gosec not detecting an expected issue and output wrongly formatted if no issue #333

Closed
andrea-tortosa opened this issue Jul 3, 2019 · 4 comments

Comments

@andrea-tortosa
Copy link

andrea-tortosa commented Jul 3, 2019

Summary

gosec sonarqube format sometimes returns

{
	"issues": null
}

when it should not because a vulnerability is in the code.
As a consequence of this SonarQube is not able to process these results and fails with

ERROR: Error during SonarQube Scanner execution
java.lang.NullPointerException
	at org.sonar.scanner.externalissue.ReportParser.validate(ReportParser.java:52)
	at org.sonar.scanner.externalissue.ReportParser.parse(ReportParser.java:43)

This gosec output is returned also if no issue is in the code but it is not correctly managed by SonarQube

Steps to reproduce the behavior

Running

gosec -fmt=sonarqube -out gosec-report.json ./... 

within a cloned repo integrated with Travis and just containing a main.go file as:

package main

import (	
	"fmt"	
	"github.com/streadway/amqp"
)

func main() {	
	var password = "f62e5bcda4fae4f82370da0c6f20697b8f8447ef"
	fmt.Printf(password)
}

it meets the error as above described. If the file is modified to

package main

import (	
	"fmt"	
	"math"
)

func main() {	
	var password = "f62e5bcda4fae4f82370da0c6f20697b8f8447ef"
	fmt.Printf(password)
}

the following is correctly returned

{
	"issues": [
		{
			"engineId": "gosec",
			"ruleId": "G101",
			"primaryLocation": {
				"message": "Potential hardcoded credentials",
				"filePath": "main.go",
				"textRange": {
					"startLine": 9,
					"endLine": 9
				}
			},
			"type": "VULNERABILITY",
			"severity": "BLOCKER",
			"effortMinutes": 5
		}
	]
}

The vulnerability should also be detected in the previous case.
Please notice that the output below

{
	"issues": null
}

is also returned when effectively there is no issue such as in the case the main.go file is

package main

import (	
	"fmt"	
	"math"
)

func main() {	
	fmt.Println(math.Exp2(10))
}

but SonarQube is not able to manage it and get again the exception above reported.
It should be the following if no issue is detected

{
	"issues": []
}

Used Sonar scanner cli version is sonar-scanner-4.0.0.1744-linux.

gosec version

Installed a few minutes ago through go get github.com/securego/gosec/cmd/gosec/...
and latest release in github.com is 2.0.0

Go version (output of 'go version')

1.12.3

Operating system / Environment

Operating System Details
Distributor ID: Ubuntu
Description: Ubuntu 16.04.6 LTS
Release: 16.04
Codename: xenial

Expected behavior

G101 vulnerability should be detected

Actual behavior

G101 vulnerability is not detected and the output file is not correctly formatted

@andrea-tortosa andrea-tortosa changed the title gosec not detecting issue and wrongly formatted if no issue gosec not detecting issue and output wrongly formatted if no issue Jul 3, 2019
@andrea-tortosa andrea-tortosa changed the title gosec not detecting issue and output wrongly formatted if no issue gosec not detecting an expected issue and output wrongly formatted if no issue Jul 3, 2019
@gcmurphy
Copy link
Member

gcmurphy commented Jul 4, 2019

There are two separate issues I see here. The detection of the vulnerability and null pointer exception that is being triggered buy "issues": null.

I have not been able to reproduce the detection issue locally but could maybe be related to the GO111MODULE=on not being present in the environment since an error was reported when you changed import to a standard library module.

The null pointer exception in Java is caused by this line. The initialization doesn't create an empty array as in this example. It should be fairly easy to fix. Will submit a PR for that soon.

gcmurphy added a commit to gcmurphy/gosec that referenced this issue Jul 4, 2019
the json encoding of uninitialized arrays is null. this causes a npe in
sonarqube tool. we should return an empty array rather than a null value
here.

relates to: securego#333
@andrea-tortosa
Copy link
Author

@gcmurphy I think I can add some info about the vulnerability detection issue. The unexpected behavior seems due to the fact that gosec is invoked through code got cloning the repo (Travis clones the repo as one of first step) and this code does not contain the dependency code which is located remotely in the not working case.
I think this is however a gosec issue which should scan the available code and not return null.
However if I include in .travis.yml a command like

- go get -d <repo to scan>/...

also the remote dependency code is downloaded and the scan works fine.

gcmurphy added a commit that referenced this issue Jul 9, 2019
* fix(formatters) null value causes npe in sonarqube

the json encoding of uninitialized arrays is null. this causes a npe in
sonarqube tool. we should return an empty array rather than a null value
here.

relates to: #333
@gcmurphy
Copy link
Member

gcmurphy commented Jul 9, 2019

The NPE issue should be fixed via #333. I think we can close this now?

@gcmurphy gcmurphy closed this as completed Jul 9, 2019
@andrea-tortosa
Copy link
Author

ok no issue is managed correctly now since the following is returned

{
	"issues": []
}

MVrachev pushed a commit to MVrachev/gosec that referenced this issue Jul 31, 2019
* fix(formatters) null value causes npe in sonarqube

the json encoding of uninitialized arrays is null. this causes a npe in
sonarqube tool. we should return an empty array rather than a null value
here.

relates to: securego#333
MVrachev pushed a commit to MVrachev/gosec that referenced this issue Jul 31, 2019
* fix(formatters) null value causes npe in sonarqube

the json encoding of uninitialized arrays is null. this causes a npe in
sonarqube tool. we should return an empty array rather than a null value
here.

relates to: securego#333
MVrachev pushed a commit to MVrachev/gosec that referenced this issue Jul 31, 2019
* fix(formatters) null value causes npe in sonarqube

the json encoding of uninitialized arrays is null. this causes a npe in
sonarqube tool. we should return an empty array rather than a null value
here.

relates to: securego#333
MVrachev pushed a commit to MVrachev/gosec that referenced this issue Jul 31, 2019
* fix(formatters) null value causes npe in sonarqube

the json encoding of uninitialized arrays is null. this causes a npe in
sonarqube tool. we should return an empty array rather than a null value
here.

relates to: securego#333
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