Reject manifests whose file entries decode far larger than their bytes (closes #123)

Before decoding the manifest, the parser adds up what decoding sets
aside for each file entry, hash, timestamp and MIME type, however short
its encoding, and refuses the manifest once that passes 8 times the
decompressed size; manifests mfer writes come to at most about 7.15
times. Empty entries decoded to about 50 times their size: under 1 KB of
manifest allocated about 500 MB. Fields the decoder does not know are
dropped; kept, they took up to 5 times more. A test refuses entries
counted just over 8 times and loads them just under. The fuzz ceiling
rises from 16 to 20 times the input and decompressed data; seeds of
empty entries and of empty hashes fail it without the fix.

Model: opus-5-5

Dockerfile

@@ -75,5 +75,7 @@ RUN version="${VERSION:-$(git describe --tags --always)}"; \
 RUN ldd /mfer 2>&1 | grep -q 'not a dynamic executable'
 
 FROM scratch
+# scratch has no CA certificates; fetch needs them to verify HTTPS servers.
+COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
 COPY --from=builder /mfer /mfer
 ENTRYPOINT ["/mfer"]

mfer/deserialize.go

@@ -267,7 +267,8 @@ func (m *manifest) deserializeInner() error {
 	// Deserialize inner message
 	m.pbInner = new(MFFile)
 
-	err = proto.Unmarshal(dat, m.pbInner)
+	// Unknown fields would cost memory; mfer never writes a loaded manifest out.
+	err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbInner)
 	if err != nil {
 		return fmt.Errorf("deserialize: unmarshal inner: %w", err)
 	}
@@ -336,7 +337,8 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) {
 	// deserialize outer:
 	m.pbOuter = new(MFFileOuter)
 
-	err = proto.Unmarshal(dat, m.pbOuter)
+	// Unknown fields would cost memory; mfer never writes a loaded manifest out.
+	err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbOuter)
 	if err != nil {
 		return nil, err
 	}

mfer/deserialize_fuzz_test.go

@@ -57,11 +57,12 @@ func FuzzNewManifestFromReader(f *testing.F) {
 		// copying, so reaching those sizes allocates up to about six times
 		// them in total. Decoding the decompressed data takes up to
 		// maxDecodedGrowth times its size for file entries, hashes,
-		// timestamps and MIME types, and up to about five times more for
+		// timestamps and MIME types, and drops fields it does not know.
-		// the bytes it copies out of it, such as fields it does not know,
+		// The strings and bytes it copies out of it, such as many one-byte
-		// which it keeps in buffers that also grow by copying. Twenty
+		// values in one hash, take up to about five times more under the
-		// times the input and the decompressed data leaves room for all of
+		// race detector, which pads every small copy to 16 bytes, and about
-		// that.
+		// half that without it. Twenty times the input and the
+		// decompressed data leaves room for all of that.
 		//
 		// The decoder also sets aside a new buffer of one to two times the
 		// window for each frame that asks for a larger window than the

mfer/deserialize_path_test.go

@@ -168,6 +168,36 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
 	}
 }
 
+// Fields the decoder does not know are dropped, in the outer message, the inner
+// message and a file entry, so that they take no memory once loaded.
+func TestDeserializeDropsUnknownFields(t *testing.T) {
+	t.Parallel()
+
+	unknown := protowire.AppendTag(nil, 99, protowire.BytesType) // in no message
+	unknown = protowire.AppendBytes(unknown, []byte("not known"))
+
+	entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
+	entry = protowire.AppendString(entry, "a")
+	entry = append(entry, unknown...)
+
+	id := uuid.New()
+	inner := protowire.AppendTag(nil, 101, protowire.BytesType) // MFFile.files
+	inner = protowire.AppendBytes(inner, entry)
+	inner = protowire.AppendTag(inner, 102, protowire.BytesType) // MFFile.uuid
+	inner = protowire.AppendBytes(inner, id[:])
+	inner = append(inner, unknown...)
+
+	data := wrapInner(t, id, inner)
+	data = append(data, unknown...) // the outer message ends the file
+
+	m, err := NewManifestFromReader(bytes.NewReader(data))
+	require.NoError(t, err)
+	require.Len(t, m.Files(), 1)
+	assert.Empty(t, m.pbOuter.ProtoReflect().GetUnknown())
+	assert.Empty(t, m.pbInner.ProtoReflect().GetUnknown())
+	assert.Empty(t, m.Files()[0].ProtoReflect().GetUnknown())
+}
+
 // Many empty files with names of at most three characters and modification
 // times at the epoch make about the densest manifest mfer writes: it takes
 // about 7 times its size to decode, and still loads. A signature would not

mfer/gpg_test.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -425,18 +426,31 @@ func TestGPGTimeoutKillsGPG(t *testing.T) {
 	assert.Contains(t, err.Error(), "gpg sign failed: gpg timed out")
 }
 
-// TestGPGTimeoutWhenChildHoldsOutput uses a fake gpg that runs sleep as a
+// TestGPGCancelWhenChildHoldsOutput uses a fake gpg that runs sleep as a
 // child instead of exec-ing it, the way a wrapper script around the real
 // gpg might. Killing the fake gpg leaves sleep holding its stdout and
-// stderr open; the call must still return shortly after the deadline
+// stderr open; the call must still return once ctx ends instead of waiting
-// instead of waiting for sleep to exit. The fake gpg writes the process ID
+// for sleep to exit. The fake gpg writes the process ID of sleep to a named
-// of sleep to a file so that the test can kill it before returning.
+// pipe; the test ends ctx only after reading it, so sleep is running by
-func TestGPGTimeoutWhenChildHoldsOutput(t *testing.T) {
+// then, and kills sleep before returning.
-	pidFile := filepath.Join(t.TempDir(), "sleep.pid")
+func TestGPGCancelWhenChildHoldsOutput(t *testing.T) {
+	pidPipe := filepath.Join(t.TempDir(), "sleep.pid")
+	require.NoError(t, syscall.Mkfifo(pidPipe, 0o600))
+	// sleep outlasts the 10 s wait below, so a call that waits for it fails.
 	t.Setenv("PATH", fakeGPGPath(t,
-		"#!/bin/sh\nsleep 3 &\necho $! >'"+pidFile+"'\nwait\n"))
+		"#!/bin/sh\nsleep 60 &\necho $! >'"+pidPipe+"'\nwait\n"))
-	t.Cleanup(func() {
+
-		pid, err := os.ReadFile(pidFile) //nolint:gosec // G304: path inside t.TempDir()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	signErr := make(chan error, 1)
+
+	go func() {
+		_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
+		signErr <- err
+	}()
+
+	pid, err := os.ReadFile(pidPipe) //nolint:gosec // G304: path inside t.TempDir()
 	require.NoError(t, err)
 
 	n, err := strconv.Atoi(strings.TrimSpace(string(pid)))
@@ -444,17 +458,19 @@ func TestGPGTimeoutWhenChildHoldsOutput(t *testing.T) {
 
 	sleep, err := os.FindProcess(n)
 	require.NoError(t, err)
-		require.NoError(t, sleep.Kill())
+	t.Cleanup(func() { require.NoError(t, sleep.Kill()) })
-	})
 
-	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	cancel()
-	defer cancel()
 
-	start := time.Now()
+	// The call should return about gpgWaitDelay (one second) after the
-	_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
+	// cancel. 10 s is far above that and well under the 30 s test timeout,
-	require.ErrorIs(t, err, context.DeadlineExceeded)
+	// which would abort the whole package before the cleanup kills sleep.
-	assert.Less(t, time.Since(start), 3*time.Second,
+	select {
-		"the call waited for the child holding gpg's output to exit")
+	case err := <-signErr:
+		require.ErrorIs(t, err, context.Canceled)
+	case <-time.After(10 * time.Second):
+		t.Fatal("the call waited for the child holding gpg's output to exit")
+	}
 }
 
 // TestBuildPassesContextToSigning checks that a caller can cancel the gpg

mfer/scanner_test.go

@@ -304,46 +304,19 @@ func TestScannerEnumerateFS(t *testing.T) {
 func TestSendEnumerateStatusNonBlocking(t *testing.T) {
 	t.Parallel()
 
-	// Channel with no buffer - send should not block
+	// Nobody receives, so a blocking send would hang the test into its timeout.
 	ch := make(chan EnumerateStatus)
 
-	// This should not block
-	done := make(chan bool)
-
-	go func() {
 	sendEnumerateStatus(ch, EnumerateStatus{FilesFound: 1})
-
-		done <- true
-	}()
-
-	select {
-	case <-done:
-		// Success - did not block
-	case <-time.After(100 * time.Millisecond):
-		t.Fatal("sendEnumerateStatus blocked on full channel")
-	}
 }
 
 func TestSendScanStatusNonBlocking(t *testing.T) {
 	t.Parallel()
 
-	// Channel with no buffer - send should not block
+	// Nobody receives, so a blocking send would hang the test into its timeout.
 	ch := make(chan ScanStatus)
 
-	done := make(chan bool)
-
-	go func() {
 	sendScanStatus(ch, ScanStatus{ScannedFiles: 1})
-
-		done <- true
-	}()
-
-	select {
-	case <-done:
-		// Success - did not block
-	case <-time.After(100 * time.Millisecond):
-		t.Fatal("sendScanStatus blocked on full channel")
-	}
 }
 
 func TestSendStatusNilChannel(t *testing.T) {