From ad792becd14a385c7545f93964e7da9cb4b248e6 Mon Sep 17 00:00:00 2001
From: Atishyy27 <142108881+Atishyy27@users.noreply.github.com>
Date: Thu, 9 Jul 2026 02:56:47 +0530
Subject: [PATCH] fix(hooks): bound OCI hook stdout/stderr capture to avoid OOM

executeHook captured a hook's stdout and stderr into plain bytes.Buffer
values, which grow without limit. A hook that streams a large amount of
data (accidentally or maliciously) could therefore exhaust urunc's
memory, since the captured output is held entirely in RAM only to build
an error message.

Replace the buffers with limitedBuffer, an io.Writer that retains at
most maxHookOutput (1 MiB) per stream and discards the rest, marking the
output as truncated. It always reports the full slice as written so the
hook process is never blocked or killed with a short-write/EPIPE error.
Add unit tests for the under-limit, exact-limit, single-oversized and
repeated-write cases.

Fixes #797

Signed-off-by: Atishyy27 <142108881+Atishyy27@users.noreply.github.com>
---
 pkg/unikontainers/utils.go      | 44 +++++++++++++++++++++++++++--
 pkg/unikontainers/utils_test.go | 49 +++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/pkg/unikontainers/utils.go b/pkg/unikontainers/utils.go
index dd9fd0b58..dc9e7b531 100644
--- a/pkg/unikontainers/utils.go
+++ b/pkg/unikontainers/utils.go
@@ -303,8 +303,46 @@ func rmMultipleDirs(prefixPath string, dirs []string) error {
 	return nil
 }
 
+// maxHookOutput caps how much stdout/stderr urunc retains from an OCI hook.
+// The captured output is only used to build error messages, so a modest cap is
+// plenty while preventing a hook that streams unbounded data from exhausting
+// memory (see #797).
+const maxHookOutput = 1 << 20 // 1 MiB per stream
+
+// limitedBuffer is an io.Writer that retains at most maxBytes bytes and discards
+// anything beyond that. It always reports the full slice as written so the
+// writing process is never blocked or killed with a short-write/EPIPE error.
+type limitedBuffer struct {
+	buf       bytes.Buffer
+	maxBytes  int
+	truncated bool
+}
+
+func (b *limitedBuffer) Write(p []byte) (int, error) {
+	if remaining := b.maxBytes - b.buf.Len(); remaining > 0 {
+		if len(p) > remaining {
+			b.buf.Write(p[:remaining])
+			b.truncated = true
+		} else {
+			b.buf.Write(p)
+		}
+	} else if len(p) > 0 {
+		b.truncated = true
+	}
+	return len(p), nil
+}
+
+// String returns the captured output, appending a marker when it was truncated.
+func (b *limitedBuffer) String() string {
+	if b.truncated {
+		return b.buf.String() + "... [output truncated]"
+	}
+	return b.buf.String()
+}
+
 func executeHook(hook specs.Hook, state []byte) error {
-	var stdout, stderr bytes.Buffer
+	stdout := &limitedBuffer{maxBytes: maxHookOutput}
+	stderr := &limitedBuffer{maxBytes: maxHookOutput}
 	var cancel context.CancelFunc
 	ctx := context.Background()
 
@@ -327,8 +365,8 @@ func executeHook(hook specs.Hook, state []byte) error {
 	cmd := exec.CommandContext(ctx, hook.Path, args...) // nolint:gosec
 	cmd.Env = hook.Env
 	cmd.Stdin = bytes.NewReader(state)
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
 
 	err := cmd.Run()
 	if err != nil {
diff --git a/pkg/unikontainers/utils_test.go b/pkg/unikontainers/utils_test.go
index b779b982e..ab2f74b8c 100644
--- a/pkg/unikontainers/utils_test.go
+++ b/pkg/unikontainers/utils_test.go
@@ -298,3 +298,52 @@ func TestLoadSpec(t *testing.T) {
 		assert.Contains(t, err.Error(), "failed to parse specification json", "Expected specific error message")
 	})
 }
+
+func TestLimitedBufferCapsOutput(t *testing.T) {
+	t.Parallel()
+
+	t.Run("under limit keeps everything", func(t *testing.T) {
+		t.Parallel()
+		b := &limitedBuffer{maxBytes: 100}
+		n, err := b.Write([]byte("hello"))
+		assert.NoError(t, err)
+		assert.Equal(t, 5, n)
+		assert.Equal(t, "hello", b.String())
+		assert.False(t, b.truncated)
+	})
+
+	t.Run("exact limit is not truncated", func(t *testing.T) {
+		t.Parallel()
+		b := &limitedBuffer{maxBytes: 3}
+		_, err := b.Write([]byte("abc"))
+		assert.NoError(t, err)
+		assert.Equal(t, "abc", b.String())
+		assert.False(t, b.truncated)
+	})
+
+	t.Run("single oversized write is capped but reports full length", func(t *testing.T) {
+		t.Parallel()
+		b := &limitedBuffer{maxBytes: 4}
+		payload := []byte("abcdefghij") // 10 bytes
+		n, err := b.Write(payload)
+		assert.NoError(t, err)
+		// Must report the full slice as written, else exec's io.Copy fails with
+		// ErrShortWrite and the hook is treated as broken.
+		assert.Equal(t, len(payload), n)
+		assert.Equal(t, 4, b.buf.Len())
+		assert.True(t, b.truncated)
+		assert.Equal(t, "abcd... [output truncated]", b.String())
+	})
+
+	t.Run("many writes stay bounded at the cap", func(t *testing.T) {
+		t.Parallel()
+		b := &limitedBuffer{maxBytes: 5}
+		for i := 0; i < 1000; i++ {
+			n, err := b.Write([]byte("xxxxxxxxxx")) // 10 bytes each
+			assert.NoError(t, err)
+			assert.Equal(t, 10, n)
+		}
+		assert.Equal(t, 5, b.buf.Len())
+		assert.True(t, b.truncated)
+	})
+}