Fileperms (#442)

Author: caccavale

README.md

@@ -82,6 +82,7 @@ directory you can supply `./...` as the input argument.
 - G303: Creating tempfile using a predictable path
 - G304: File path provided as taint input
 - G305: File traversal when extracting zip archive
+- G306: Poor file permissions used when writing to a new file
 - 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

cmd/tlsconfig/tlsconfig.go

@@ -186,5 +186,5 @@ func main() {
 	outputPath := filepath.Join(dir, *outputFile)
 	if err := ioutil.WriteFile(outputPath, src, 0644); err != nil {
 		log.Fatalf("Writing output: %s", err)
-	}
+	} // #nosec G306
 }

rules/fileperms.go

@@ -60,6 +60,22 @@ func (r *filePermissions) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, err
 	return nil, nil
 }
 
+// NewWritePerms creates a rule to detect file Writes with bad permissions.
+func NewWritePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
+	mode := getConfiguredMode(conf, "G306", 0600)
+	return &filePermissions{
+		mode:  mode,
+		pkg:   "io/ioutil",
+		calls: []string{"WriteFile"},
+		MetaData: gosec.MetaData{
+			ID:         id,
+			Severity:   gosec.Medium,
+			Confidence: gosec.High,
+			What:       fmt.Sprintf("Expect WriteFile permissions to be %#o or less", mode),
+		},
+	}, []ast.Node{(*ast.CallExpr)(nil)}
+}
+
 // NewFilePerms creates a rule to detect file creation with a more permissive than configured
 // permission mask.
 func NewFilePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {

rules/rulelist.go

@@ -81,6 +81,7 @@ func Generate(filters ...RuleFilter) RuleList {
 		{"G303", "Creating tempfile using a predictable path", NewBadTempFile},
 		{"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},
 
 		// crypto
 		{"G401", "Detect the usage of DES, RC4, MD5 or SHA1", NewUsesWeakCryptography},

rules/rules_test.go

@@ -127,6 +127,10 @@ var _ = Describe("gosec rules", func() {
 			runner("G305", testutils.SampleCodeG305)
 		})
 
+		It("should detect poor permissions when writing to a file", func() {
+			runner("G306", testutils.SampleCodeG306)
+		})
+
 		It("should detect weak crypto algorithms", func() {
 			runner("G401", testutils.SampleCodeG401)
 		})

testutils/pkg.go

@@ -55,7 +55,7 @@ func (p *TestPackage) write() error {
 	for filename, content := range p.Files {
 		if e := ioutil.WriteFile(filename, []byte(content), 0644); e != nil {
 			return e
-		}
+		} // #nosec G306
 	}
 	p.ondisk = true
 	return nil

testutils/source.go

@@ -1400,6 +1400,55 @@ func unzip(archive, target string) error {
 	}
 
 	return nil
+}`}, 1, gosec.NewConfig()}}
+
+	// SampleCodeG306 - Poor permissions for WriteFile
+	SampleCodeG306 = []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)
+	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()}}
 
 	// SampleCodeG401 - Use of weak crypto MD5