From aee7d3fe355dd02939d44155e308ea0052e0d53a Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <cyphar@cyphar.com>
Date: Thu, 15 May 2025 17:40:25 +1000
Subject: [PATCH] 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 <cyphar@cyphar.com>
---
 .golangci.yml              | 15 +++++++++++++++
 libcontainer/criu_linux.go |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/.golangci.yml b/.golangci.yml
index 5d31ed92a..b9a5bc7bf 100644
--- a/.golangci.yml
+++ b/.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
diff --git a/libcontainer/criu_linux.go b/libcontainer/criu_linux.go
index fb435e111..e96873923 100644
--- a/libcontainer/criu_linux.go
+++ b/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
 		}