Generated SARIF file is not valid SARIF

[v-homsi]

Summary

Generated SARIF file is not considered valid as per upload-sarif github action.

This is due to the duplicate ids of some of the rules generated in the SARIF file.

Steps to reproduce the behavior

To reproduce the error, you will need to run a github action similar to this. You can also check out the error in this run.

To reproduce the SARIF file, you can run:

git clone https://github.com/v-homsi/evmos.git
git checkout fix/gosec
go install github.com/cosmos/gosec/v2/cmd/gosec
gosec --no-fail -fmt sarif -out results.sarif ./...

gosec version

Master branch

Go version (output of 'go version')

go1.18.4

Operating system / Environment

Ubuntu

Expected behavior

Generated SARIF is valid and can be uploaded

Actual behavior

Generated SARIF is not valid and cannot be uploaded

I cannot upload the sample file as the format is not supported by github. But here's a snippet of the SARIF file generated:

"tool": {
    "driver": {
        "name": "gosec",
        "informationUri": "https://github.com/securego/gosec/",
        "rules": [
            {
                "id": "G701 (CWE-)",
                "name": "Potential integer overflow by integer type conversion",
                "shortDescription": {
                    "text": "Potential integer overflow by integer type conversion"
                },
                "fullDescription": {
                    "text": "Potential integer overflow by integer type conversion"
                },
                "help": {
                    "text": "Potential integer overflow by integer type conversion\nSeverity: HIGH\nConfidence: MEDIUM\nCWE: "
                },
                "properties": {
                    "tags": [
                        "CWE-",
                        "HIGH"
                    ]
                }
            },
            {
                "id": "G701 (CWE-)",
                "name": "Potential integer overflow by integer type conversion",
                "shortDescription": {
                    "text": "Potential integer overflow by integer type conversion"
                },
                "fullDescription": {
                    "text": "Potential integer overflow by integer type conversion"
                },
                "help": {
                    "text": "Potential integer overflow by integer type conversion\nSeverity: HIGH\nConfidence: MEDIUM\nCWE: "
                },
                "properties": {
                    "tags": [
                        "CWE-",
                        "HIGH"
                    ]
                }
            },
            {
                "id": "G701 (CWE-)",
                "name": "Potential integer overflow by integer type conversion",
                "shortDescription": {
                    "text": "Potential integer overflow by integer type conversion"
                },
                "fullDescription": {
                    "text": "Potential integer overflow by integer type conversion"
                },
                "help": {
                    "text": "Potential integer overflow by integer type conversion\nSeverity: HIGH\nConfidence: MEDIUM\nCWE: "
                },
                "properties": {
                    "tags": [
                        "CWE-",
                        "HIGH"
                    ]
                }
            }
        ]
    }
},

[odeke-em]

Thank you for the report @v-homsi! Kindly tagging my colleague @kirbyquerby to take a look.

[kirbyquerby]

It looks like this was previously fixed in the upstream securego/gosec in securego/gosec#565

I've made #39 to pull in those changes, which should hopefully solve the issue. This repo is based on a pretty old version of gosec (almost 2 years old, I believe). Since this repository is primarily just additional rules on top of what securego/gosec provides, we would probably benefit a lot from figuring out a way to just add our rules to the latest version of gosec, rather than maintaining an entire fork. This will let us benefit from bug fixes, features, etc from upstream instead of having to discover and fix them separately ourselves.

[odeke-em]

Roger that @kirbyquerby and thank you for investigating!

[kirbyquerby]

@v-homsi this has hopefully been fixed by #39. I've released v0.0.4.

I'm closing this as the action now succeeds in my fork: https://github.com/kirbyquerby/evmos/actions/runs/3063781539/jobs/4946236000

Feel free to reopen if something's still not working.

[v-homsi]

@kirbyquerby @odeke-em Works perfectly! Thanks for the help on this! Much appreciated :)