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

Remove rule G307 which checks when an error is not handled when a file or socket connection is closed #935

Merged
merged 4 commits into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ directory you can supply `./...` as the input argument.
- G304: File path provided as taint input
- G305: File traversal when extracting zip/tar archive
- G306: Poor file permissions used when writing to a new file
- G307: Deferring a method which returns an error
- G401: Detect the usage of DES, RC4, MD5 or SHA1
- G402: Look for bad TLS connection settings
- G403: Ensure minimum RSA key length of 2048 bits
Expand All @@ -172,6 +171,7 @@ directory you can supply `./...` as the input argument.
### Retired rules

- G105: Audit the use of math/big.Int.Exp - [CVE is fixed](https://github.com/golang/go/issues/15184)
- G307: Deferring a method which returns an error - causing more inconvenience than fixing a security issue, despite the details from this [blog post](https://www.joeshaw.org/dont-defer-close-on-writable-files/)

### Selecting rules

Expand Down
1 change: 0 additions & 1 deletion issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ var ruleToCWE = map[string]string{
"G304": "22",
"G305": "22",
"G306": "276",
"G307": "703",
"G401": "326",
"G402": "295",
"G403": "310",
Expand Down
97 changes: 0 additions & 97 deletions rules/bad_defer.go

This file was deleted.

1 change: 0 additions & 1 deletion rules/rulelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList {
{"G304", "File path provided as taint input", NewReadFile},
{"G305", "File path traversal when extracting zip archive", NewArchive},
{"G306", "Poor file permissions used when writing to a file", NewWritePerms},
{"G307", "Unsafe defer call of a method returning an error", NewDeferredClosing},

// crypto
{"G401", "Detect the usage of DES, RC4, MD5 or SHA1", NewUsesWeakCryptography},
Expand Down
4 changes: 0 additions & 4 deletions rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@ var _ = Describe("gosec rules", func() {
runner("G306", testutils.SampleCodeG306)
})

It("should detect unsafe defer of os.Close", func() {
runner("G307", testutils.SampleCodeG307)
})

It("should detect weak crypto algorithms", func() {
runner("G401", testutils.SampleCodeG401)
})
Expand Down
52 changes: 0 additions & 52 deletions testutils/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -2777,58 +2777,6 @@ func main() {

}`}, 1, gosec.NewConfig()},
}
// SampleCodeG307 - Unsafe defer of os.Close
SampleCodeG307 = []CodeSample{
{[]string{`package main
import (
"bufio"
"fmt"
"io/ioutil"
"os"
)
func check(e error) {
if e != nil {
panic(e)
}
}
func main() {
d1 := []byte("hello\ngo\n")
err := ioutil.WriteFile("/tmp/dat1", d1, 0744)
check(err)
allowed := ioutil.WriteFile("/tmp/dat1", d1, 0600)
check(allowed)
f, err := os.Create("/tmp/dat2")
check(err)
defer f.Close()
d2 := []byte{115, 111, 109, 101, 10}
n2, err := f.Write(d2)
defer check(err)
fmt.Printf("wrote %d bytes\n", n2)
n3, err := f.WriteString("writes\n")
fmt.Printf("wrote %d bytes\n", n3)
f.Sync()
w := bufio.NewWriter(f)
n4, err := w.WriteString("buffered\n")
fmt.Printf("wrote %d bytes\n", n4)
w.Flush()
}`}, 1, gosec.NewConfig()}, {[]string{`
package main

import (
"net"
"net/http"
)

func main() {
response, _ := http.Get("https://127.0.0.1")

defer response.Body.Close() // io.ReadCloser

conn, _ := net.Dial("tcp", "127.0.0.1:8080")
defer conn.Close() // net.Conn

}`}, 2, gosec.NewConfig()},
}

// SampleCodeG401 - Use of weak crypto MD5
SampleCodeG401 = []CodeSample{
Expand Down