Updates to a few Go rules

go/lang/correctness/permissions/file_permission.fixed.go

@@ -0,0 +1,73 @@
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+)
+
+func main() {
+}
+
+func test_chmod() {
+	// ruleid: incorrect-default-permission
+	err := os.Chmod("/tmp/somefile", 0600)
+	if err != nil {
+		fmt.Println("Error when changing file permissions!")
+		return
+	}
+
+	// ok: incorrect-default-permission
+	err := os.Chmod("/tmp/somefile", 0400)
+	if err != nil {
+		fmt.Println("Error when changing file permissions!")
+		return
+	}
+}
+
+func test_mkdir() {
+	// ruleid: incorrect-default-permission
+	err := os.Mkdir("/tmp/mydir", 0600)
+	if err != nil {
+		fmt.Println("Error when creating a directory!")
+		return
+	}
+
+	// ruleid: incorrect-default-permission
+	err = os.MkdirAll("/tmp/mydir", 0600)
+	if err != nil {
+		fmt.Println("Error when creating a directory!")
+		return
+	}
+
+	// ok: incorrect-default-permission
+	err := os.MkdirAll("/tmp/mydir", 0600)
+	if err != nil {
+		fmt.Println("Error when creating a directory!")
+		return
+	}
+}
+
+func test_openfile() {
+	// ruleid: incorrect-default-permission
+	_, err := os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0600)
+	if err != nil {
+		fmt.Println("Error opening a file!")
+		return
+	}
+
+	// ok: incorrect-default-permission
+	_, err := os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0600)
+	if err != nil {
+		fmt.Println("Error opening a file!")
+		return
+	}
+}
+
+func test_writefile() {
+	// ruleid: incorrect-default-permission
+	err := ioutil.WriteFile("/tmp/demo2", []byte("This is some data"), 0600)
+	if err != nil {
+		fmt.Println("Error while writing!")
+	}
+}

go/lang/correctness/permissions/file_permission.go

@@ -34,7 +34,7 @@ func test_mkdir() {
 	}
 
 	// ruleid: incorrect-default-permission
-	err := os.MkdirAll("/tmp/mydir", 0777)
+	err = os.MkdirAll("/tmp/mydir", 0777)
 	if err != nil {
 		fmt.Println("Error when creating a directory!")
 		return

go/lang/correctness/permissions/file_permission.yaml

@@ -23,3 +23,7 @@ rules:
           metavariable: $PERM
           comparison: $PERM > 0o600
           base: 8
+      - focus-metavariable:
+          - $PERM
+    fix: |
+      0600

go/lang/security/audit/crypto/bad_imports.yaml

@@ -1,8 +1,8 @@
 rules:
 - id: insecure-module-used
   message: >-
-    Detected use of an insecure cryptographic hashing method. This method is known
-    to be broken and easily compromised. Use SHA256 or SHA3 instead.
+    The package `net/http/cgi` is on the import blocklist. 
+    The package is vulnerable to httpoxy attacks (CVE-2015-5386).
   metadata:
     owasp:
     - A03:2017 - Sensitive Data Exposure
@@ -23,30 +23,6 @@ rules:
   languages: [go]
   severity: WARNING
   pattern-either:
-  - patterns:
-    - pattern-inside: |
-        import "crypto/md5"
-        ...
-    - pattern: |
-        md5.$FUNC(...)
-  - patterns:
-    - pattern-inside: |
-        import "crypto/des"
-        ...
-    - pattern: |
-        des.$FUNC(...)
-  - patterns:
-    - pattern-inside: |
-        import "crypto/sha1"
-        ...
-    - pattern: |
-        sha1.$FUNC(...)
-  - patterns:
-    - pattern-inside: |
-        import "crypto/rc4"
-        ...
-    - pattern: |
-        rc4.$FUNC(...)
   - patterns:
     - pattern-inside: |
         import "net/http/cgi"

go/lang/security/audit/crypto/math_random.fixed.go

@@ -0,0 +1,40 @@
+package main
+
+import (
+	"crypto/rand"
+	// ruleid: math-random-used
+	mrand "crypto/rand"
+)
+
+func main() {
+	main0()
+	main1()
+	main2()
+	main3()
+}
+
+func main0() {
+	// ok: math-random-used
+	bad, _ := mrand.Read(nil)
+	println(bad)
+}
+
+func main1() {
+	// ok: math-random-used
+	good, _ := rand.Read(nil)
+	println(good)
+}
+
+func main2() {
+	// ok: math-random-used
+	bad := mrand.Int()
+	println(bad)
+}
+
+func main3() {
+	// ok: math-random-used
+	good, _ := rand.Read(nil)
+	println(good)
+	i := mrand.Int31()
+	println(i)
+}

go/lang/security/audit/crypto/math_random.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"crypto/rand"
+	// ruleid: math-random-used
 	mrand "math/rand"
 )
 
@@ -13,7 +14,7 @@ func main() {
 }
 
 func main0() {
-	// ruleid: math-random-used
+	// ok: math-random-used
 	bad, _ := mrand.Read(nil)
 	println(bad)
 }
@@ -25,7 +26,7 @@ func main1() {
 }
 
 func main2() {
-	// ruleid: math-random-used
+	// ok: math-random-used
 	bad := mrand.Int()
 	println(bad)
 }

go/lang/security/audit/crypto/math_random.yaml

@@ -18,21 +18,23 @@ rules:
   message: Do not use `math/rand`. Use `crypto/rand` instead.
   languages: [go]
   severity: WARNING
-  pattern-either:
-  - patterns:
-    - pattern-inside: |
-        import mrand "math/rand"
-        ...
+  patterns:
     - pattern-either:
-      - pattern: mrand.Int()
-      - pattern: mrand.Read(...)
-  - patterns:
-    - pattern-inside: |
-        import "math/rand"
-        ...
-    - pattern-not-inside: |
-        import "crypto/rand"
-        ...
+      - pattern: |
+          import $RAND "$MATH"
+      - pattern: |
+          import "$MATH"
+    - metavariable-regex:
+        metavariable: $MATH
+        regex: ^(math/rand)$
     - pattern-either:
-      - pattern: rand.Int()
-      - pattern: rand.Read(...)
+        - pattern-inside: |
+            ...
+            rand.$FUNC(...)
+        - pattern-inside: |
+            ...
+            $RAND.$FUNC(...)
+    - focus-metavariable:
+        - $MATH
+  fix: |
+    crypto/rand

go/lang/security/audit/crypto/missing-ssl-minversion.fixed.go

@@ -0,0 +1,68 @@
+package main
+
+import (
+	"crypto/tls"
+	"log"
+	"net/http"
+	"net/http/httptest"
+	"os"
+)
+
+// zeroSource is an io.Reader that returns an unlimited number of zero bytes.
+type zeroSource struct{}
+
+func (zeroSource) Read(b []byte) (n int, err error) {
+	for i := range b {
+		b[i] = 0
+	}
+
+	return len(b), nil
+}
+
+func main() {
+	// Dummy test HTTP server for the example with insecure random so output is
+	// reproducible.
+	server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	// ruleid: missing-ssl-minversion
+	server.TLS = &tls.Config{ Rand: zeroSource{}, MinVersion: tls.VersionTLS13 }
+	server.StartTLS()
+	defer server.Close()
+
+	// Typically the log would go to an open file:
+	// w, err := os.OpenFile("tls-secrets.txt", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+	w := os.Stdout
+
+	client := &http.Client{
+		Transport: &http.Transport{
+			// ok: missing-ssl-minversion
+			TLSClientConfig: &tls.Config{
+				KeyLogWriter:       w,
+				MinVersion:         tls.VersionSSL30,
+				Rand:               zeroSource{}, // for reproducible output; don't do this.
+				InsecureSkipVerify: true,         // test server certificate is not trusted.
+			},
+		},
+	}
+	resp, err := client.Get(server.URL)
+	if err != nil {
+		log.Fatalf("Failed to get URL: %v", err)
+	}
+	resp.Body.Close()
+
+	clientGood := &http.Client{
+		Transport: &http.Transport{
+			// ok: missing-ssl-minversion
+			TLSClientConfig: &tls.Config{
+				KeyLogWriter: w,
+				MinVersion:         tls.VersionTLS10,
+				Rand:               zeroSource{}, // for reproducible output; don't do this.
+				InsecureSkipVerify: true,         // test server certificate is not trusted.
+			},
+		},
+	}
+	resp, err := client.Get(server.URL)
+	if err != nil {
+		log.Fatalf("Failed to get URL: %v", err)
+	}
+	resp.Body.Close()
+}

go/lang/security/audit/crypto/missing-ssl-minversion.yaml

@@ -1,9 +1,11 @@
 rules:
 - id: missing-ssl-minversion
   message: >-
-    `MinVersion` is missing from this TLS configuration. The default
-    value is TLS1.0 which is considered insecure. Explicitly set the
-    `MinVersion` to a secure version of TLS, such as `VersionTLS13`.
+    `MinVersion` is missing from this TLS configuration. 
+    By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server.
+    General purpose web applications should default to TLS 1.3 with all other protocols disabled. 
+    Only where it is known that a web server must support legacy clients
+    with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support.
   metadata:
     cwe:
     - 'CWE-327: Use of a Broken or Risky Cryptographic Algorithm'
@@ -20,16 +22,16 @@ rules:
     - go
     confidence: HIGH
     subcategory:
-    - audit
+    - guardrail
     likelihood: MEDIUM
     impact: LOW
   languages: [go]
   severity: WARNING
   patterns:
-  - pattern: |-
-      tls.Config{...}
-  - pattern-not-inside: |-
+  - pattern: |
+      tls.Config{ $...CONF }
+  - pattern-not: |
       tls.Config{..., MinVersion: ..., ...}
-  fix-regex:
-    regex: Config\s*\{
-    replacement: 'Config{MinVersion: SSL.VersionTLS13,'
+  fix: |
+    tls.Config{ $...CONF, MinVersion: tls.VersionTLS13 }
+