ci: add lint to forbid the usage of os.Create

os.Create is shorthand for open(O_CREAT|O_TRUNC) *without* O_EXCL, which
is incredibly unsafe for us to do when interacting with a container
rootfs (especially before pivot_root) as an attacker could swap the
target path with a symlink that points to the host filesystem, causing
us to delete the contents of or create host files.

We did have a similar bug in CVE-2024-45310, but in that case we
(luckily) didn't have O_TRUNC set which avoided the worst possible case.
However, os.Create does set O_TRUNC and we were using it in scenarios
that may have been exploitable.

Because of how easy it us for us to accidentally introduce this kind of
bug, we should simply not allow the usage of os.Create in our entire
codebase.

Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

.golangci.yml

@@ -11,6 +11,7 @@ formatters:
 linters:
   enable:
     - errorlint
+    - forbidigo
     - nolintlint
     - unconvert
     - unparam
@@ -25,6 +26,20 @@ linters:
         - -ST1003 # https://staticcheck.dev/docs/checks/#ST1003 Poorly chosen identifier.
         - -ST1005 # https://staticcheck.dev/docs/checks/#ST1005 Incorrectly formatted error string.
         - -QF1008 # https://staticcheck.dev/docs/checks/#QF1008 Omit embedded fields from selector expression.
+    forbidigo:
+      forbid:
+        # os.Create implies O_TRUNC without O_CREAT|O_EXCL, which can lead to
+        # an even more severe attacks than CVE-2024-45310, where host files
+        # could be wiped. Always use O_EXCL or otherwise ensure we are not
+        # going to be tricked into overwriting host files.
+        - pattern: ^os\.Create$
+          pkg: ^os$
+      analyze-types: true
   exclusions:
+    rules:
+      # forbidigo lints are only relevant for main code.
+      - path: '(.+)_test\.go'
+        linters:
+          - forbidigo
     presets:
       - std-error-handling

libcontainer/criu_linux.go

@@ -1090,7 +1090,7 @@ func (c *Container) criuNotifications(resp *criurpc.CriuResp, process *Process,
 	logrus.Debugf("notify: %s\n", script)
 	switch script {
 	case "post-dump":
-		f, err := os.Create(filepath.Join(c.stateDir, "checkpoint"))
+		f, err := os.Create(filepath.Join(c.stateDir, "checkpoint")) //nolint:forbidigo // this is a host-side operation in a runc-controlled directory
 		if err != nil {
 			return err
 		}