Add gosec linter (#1342)

This PR adds `gosec` linter with the following checks disabled:

- G102: Bind to all interfaces
- G107: Url provided to HTTP request as taint input
- G112: Potential slowloris attack
- G114: Use of net/http serve function that has no support for setting timeouts
- G204: Audit use of command execution
- G401: Detect the usage of DES, RC4, MD5 or SHA1
- G402: Look for bad TLS connection settings
- G404: Insecure random number source (rand)
- G501: Import blocklist: crypto/md5
- G505: Import blocklist: crypto/sha1

We have complaints related to the checks above. They have to be addressed separately.

.golangci.yaml

@@ -12,6 +12,44 @@ linters-settings:
     # Default: false
     check-type-assertions: false
 
+  gosec:
+    includes:
+      - G101 # Look for hard coded credentials
+      #- G102 # Bind to all interfaces
+      - G103 # Audit the use of unsafe block
+      - G104 # Audit errors not checked
+      - G106 # Audit the use of ssh.InsecureIgnoreHostKey
+      #- G107 # Url provided to HTTP request as taint input
+      - G108 # Profiling endpoint automatically exposed on /debug/pprof
+      - G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32
+      - G110 # Potential DoS vulnerability via decompression bomb
+      - G111 # Potential directory traversal
+      #- G112 # Potential slowloris attack
+      - G113 # Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
+      #- G114 # Use of net/http serve function that has no support for setting timeouts
+      - G201 # SQL query construction using format string
+      - G202 # SQL query construction using string concatenation
+      - G203 # Use of unescaped data in HTML templates
+      #- G204 # Audit use of command execution
+      - G301 # Poor file permissions used when creating a directory
+      - G302 # Poor file permissions used with chmod
+      - G303 # Creating tempfile using a predictable path
+      - 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 # Poor file permissions used when creating a file with os.Create
+      #- 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
+      #- G404 # Insecure random number source (rand)
+      #- G501 # Import blocklist: crypto/md5
+      - G502 # Import blocklist: crypto/des
+      - G503 # Import blocklist: crypto/rc4
+      - G504 # Import blocklist: net/http/cgi
+      #- G505 # Import blocklist: crypto/sha1
+      - G601 # Implicit memory aliasing of items from a range statement
+      - G602 # Slice access out of bounds
+
   gocritic:
     disabled-checks:
       - commentFormatting
@@ -49,6 +87,7 @@ linters:
     - durationcheck # durationcheck checks for two durations multiplied together
     - forbidigo # forbidigo forbids identifiers
     - gocritic # provides diagnostics that check for bugs, performance and style issues
+    - gosec # inspects source code for security problems
     - mirror # mirror reports wrong mirror patterns of bytes/strings usage
     - misspell # misspess finds commonly misspelled English words in comments
     - nilerr # finds the code that returns nil even if it checks that the error is not nil
@@ -65,19 +104,20 @@ issues:
 
   exclude-rules:
     # allow fmt
-    - path: management/cmd/root.go
+    - path: management/cmd/root\.go
       linters: forbidigo
-    - path: signal/cmd/root.go
+    - path: signal/cmd/root\.go
       linters: forbidigo
-    - path: sharedsock/filter.go
+    - path: sharedsock/filter\.go
       linters:
       - unused
-    - path: client/firewall/iptables/rule.go
+    - path: client/firewall/iptables/rule\.go
       linters:
       - unused
-    - path: test.go
+    - path: test\.go
       linters:
       - mirror
-    - path: mock.go
+      - gosec
+    - path: mock\.go
       linters:
       - nilnil

client/firewall/uspfilter/uspfilter.go

@@ -355,14 +355,16 @@ func (m *Manager) RemovePacketHook(hookID string) error {
 	for _, arr := range m.incomingRules {
 		for _, r := range arr {
 			if r.id == hookID {
-				return m.DeleteRule(&r)
+				rule := r
+				return m.DeleteRule(&rule)
 			}
 		}
 	}
 	for _, arr := range m.outgoingRules {
 		for _, r := range arr {
 			if r.id == hookID {
-				return m.DeleteRule(&r)
+				rule := r
+				return m.DeleteRule(&rule)
 			}
 		}
 	}

client/internal/routemanager/systemops_linux.go

@@ -107,7 +107,8 @@ loop:
 			break loop
 		case syscall.RTM_NEWROUTE:
 			rt := (*routeInfoInMemory)(unsafe.Pointer(&m.Data[0]))
-			attrs, err := syscall.ParseNetlinkRouteAttr(&m)
+			msg := m
+			attrs, err := syscall.ParseNetlinkRouteAttr(&msg)
 			if err != nil {
 				return nil, err
 			}
@@ -146,5 +147,5 @@ func enableIPForwarding() error {
 		return nil
 	}
 
-	return os.WriteFile(ipv4ForwardingPath, []byte("1"), 0644)
+	return os.WriteFile(ipv4ForwardingPath, []byte("1"), 0644) //nolint:gosec
 }

client/ui/client_ui.go

@@ -634,5 +634,5 @@ func checkPIDFile() error {
 		}
 	}
 
-	return os.WriteFile(pidFile, []byte(fmt.Sprintf("%d", os.Getpid())), 0o664)
+	return os.WriteFile(pidFile, []byte(fmt.Sprintf("%d", os.Getpid())), 0o664) //nolint:gosec
 }

iface/tun_linux.go

@@ -99,7 +99,8 @@ func (c *tunDevice) assignAddr() error {
 	}
 	if len(list) > 0 {
 		for _, a := range list {
-			err = netlink.AddrDel(link, &a)
+			addr := a
+			err = netlink.AddrDel(link, &addr)
 			if err != nil {
 				return err
 			}

management/server/http/policies_handler.go

@@ -290,11 +290,13 @@ func toPolicyResponse(account *server.Account, policy *server.Policy) *api.Polic
 		Enabled:     policy.Enabled,
 	}
 	for _, r := range policy.Rules {
+		rID := r.ID
+		rDescription := r.Description
 		rule := api.PolicyRule{
-			Id:            &r.ID,
+			Id:            &rID,
 			Name:          r.Name,
 			Enabled:       r.Enabled,
-			Description:   &r.Description,
+			Description:   &rDescription,
 			Bidirectional: r.Bidirectional,
 			Protocol:      api.PolicyRuleProtocol(r.Protocol),
 			Action:        api.PolicyRuleAction(r.Action),

management/server/metrics/selfhosted.go

@@ -390,14 +390,18 @@ func getMinMaxVersion(inputList []string) (string, string) {
 			}
 		}
 	}
-	switch len(versions) {
+
+	targetIndex := 1
+	l := len(versions)
+
+	switch l {
 	case 0:
 		return "", ""
-	case 1:
-		v := versions[0].String()
+	case targetIndex:
+		v := versions[targetIndex-1].String()
 		return v, v
 	default:
 		sort.Sort(version.Collection(versions))
-		return versions[0].String(), versions[len(versions)-1].String()
+		return versions[targetIndex-1].String(), versions[l-1].String()
 	}
 }