From af11e85a3841d6a2fc882a3487c75bc6897cbf6e 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 954a2bc00..1a802316d 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -11,6 +11,7 @@ formatters:
 linters:
   enable:
     - errorlint
+    - forbidigo
     - unconvert
     - unparam
   settings:
@@ -24,6 +25,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 fd4b204a9..17658a4d3 100644
--- a/libcontainer/criu_linux.go
+++ b/libcontainer/criu_linux.go
@@ -1077,7 +1077,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
 		}