v2.9.6 ignores all nosec annotations

[tim-lo]

Summary

The latest version of gosec seems to ignore all nosec annotations for rule G201. Reverting to v2.9.5 fixed my issue though, so something's borked with v2.9.6 I imagine? I don't have any global gosec configurations.

This doesn't work:

query := fmt.Sprintf("SELECT * FROM %s", tableName) //#nosec G201 -- constant value, not user input

Even this doesn't do anything:

/* #nosec */
query := fmt.Sprintf("SELECT * FROM %s", tableName)

Steps to reproduce the behavior

Add nosec annotation to format string-constructed sql query

gosec version

2.9.6

Go version (output of 'go version')

1.17.6

Operating system / Environment

macOS 12.1

Expected behavior

Gosec should ignore G201 false positives

Actual behavior

Gosec doesn't ignore G201 false positives

[ccojocar]

@Yiwei-Ding Please could you have a look at this issue? Thanks

[Yiwei-Ding]

@ccojocar this is similar to #742. Take this piece of code as an example:

package main
import (
    "database/sql"
    "fmt"
    "os"
)
func main(){
    db, err := sql.Open("sqlite3", ":memory:")
    if err != nil {
        panic(err)
    }
    q := fmt.Sprintf("SELECT * FROM foo where name = '%s'", os.Args[1])
    rows, err := db.Query(q)
    if err != nil {
        panic(err)
    }
    defer rows.Close()
}

If we use #nosec for q := fmt.Sprintf("...", ...), we could find it doesn't work. Instead, if we put #nosec for db.Query(q), we will find it could suppress the violation:

package main
import (
    "database/sql"
    "fmt"
    "os"
)
func main(){
    db, err := sql.Open("sqlite3", ":memory:")
    if err != nil {
        panic(err)
    }
    q := fmt.Sprintf("SELECT * FROM foo where name = '%s'", os.Args[1])
    rows, err := db.Query(q) //#nosec G201 -- this is a fp.
    if err != nil {
        panic(err)
    }
    defer rows.Close()
}

The root issue would be bug on G201. It tells the location of construction of the string instead of using the string for query. What's your idea?

@tim-lo Sorry but it doesn't work with v2.9.5 on my machine as well...

[ccojocar]

Thanks for looking into this. We should probably fix the rule to report the exact location where the issue is.

[ccampo133]

Just ran into this, specifically for G201, using the latest version (v2.13.1). Confirmed that @Yiwei-Ding's workaround (adding the comment before the call to Query instead of the actual string format) does indeed work. I recommend updating the title of this issue to specifically call out G201.

[sevein]

For your information, I've noticed that when going from v2.18.0 to v2.18.1that in the following snippet the #nosec directive is ignored:

err = os.MkdirAll(p, 0o775); if err != nil { //#nosec G301
	return err
}

But this works:

err = os.MkdirAll(p, 0o775) //#nosec G301
if err != nil {
	return err
}

[ccojocar]

This is fixed.