1 Commits
Author SHA1 Message Date
sneak 10ad90ba22 Reject manifests whose file entries decode far larger than their bytes (closes #123)
check / check (push) Failing after 1s
Parser fix: before decoding the manifest, the parser walks its file
entries and adds up what decoding sets aside for each entry, hash,
timestamp and MIME type, however short its encoding. It refuses the
manifest once that sum 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, so a manifest of under 1 KB allocated about
500 MB. A test refuses entries counted at just over 8 times and loads
them at just under. The fuzz target's ceiling rises from 16 to 20 times
the input and decompressed data, and seeds of empty entries and of
empty hashes fail it without the fix.

Model: opus-5-5
2026-10-04 07:38:09 +00:00
6 changed files with 54 additions and 78 deletions

No files matched your search

-2
View File
@@ -75,7 +75,5 @@ RUN version="${VERSION:-$(git describe --tags --always)}"; \
RUN ldd /mfer 2>&1 | grep -q 'not a dynamic executable' RUN ldd /mfer 2>&1 | grep -q 'not a dynamic executable'
FROM scratch 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 COPY --from=builder /mfer /mfer
ENTRYPOINT ["/mfer"] ENTRYPOINT ["/mfer"]
+2 -4
View File
@@ -267,8 +267,7 @@ func (m *manifest) deserializeInner() error {
// Deserialize inner message // Deserialize inner message
m.pbInner = new(MFFile) m.pbInner = new(MFFile)
// Unknown fields would cost memory; mfer never writes a loaded manifest out. err = proto.Unmarshal(dat, m.pbInner)
err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbInner)
if err != nil { if err != nil {
return fmt.Errorf("deserialize: unmarshal inner: %w", err) return fmt.Errorf("deserialize: unmarshal inner: %w", err)
} }
@@ -337,8 +336,7 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) {
// deserialize outer: // deserialize outer:
m.pbOuter = new(MFFileOuter) m.pbOuter = new(MFFileOuter)
// Unknown fields would cost memory; mfer never writes a loaded manifest out. err = proto.Unmarshal(dat, m.pbOuter)
err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbOuter)
if err != nil { if err != nil {
return nil, err return nil, err
} }
+5 -6
View File
@@ -57,12 +57,11 @@ func FuzzNewManifestFromReader(f *testing.F) {
// copying, so reaching those sizes allocates up to about six times // copying, so reaching those sizes allocates up to about six times
// them in total. Decoding the decompressed data takes up to // them in total. Decoding the decompressed data takes up to
// maxDecodedGrowth times its size for file entries, hashes, // maxDecodedGrowth times its size for file entries, hashes,
// timestamps and MIME types, and drops fields it does not know. // timestamps and MIME types, and up to about five times more for
// The strings and bytes it copies out of it, such as many one-byte // the bytes it copies out of it, such as fields it does not know,
// values in one hash, take up to about five times more under the // which it keeps in buffers that also grow by copying. Twenty
// race detector, which pads every small copy to 16 bytes, and about // times the input and the decompressed data leaves room for all of
// half that without it. Twenty times the input and the // that.
// decompressed data leaves room for all of that.
// //
// The decoder also sets aside a new buffer of one to two times the // 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 // window for each frame that asks for a larger window than the
-30
View File
@@ -168,36 +168,6 @@ 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 // 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 // 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 // about 7 times its size to decode, and still loads. A signature would not
+18 -34
View File
@@ -10,7 +10,6 @@ import (
"path/filepath" "path/filepath"
"strconv" "strconv"
"strings" "strings"
"syscall"
"testing" "testing"
"time" "time"
@@ -426,31 +425,18 @@ func TestGPGTimeoutKillsGPG(t *testing.T) {
assert.Contains(t, err.Error(), "gpg sign failed: gpg timed out") assert.Contains(t, err.Error(), "gpg sign failed: gpg timed out")
} }
// TestGPGCancelWhenChildHoldsOutput uses a fake gpg that runs sleep as a // TestGPGTimeoutWhenChildHoldsOutput uses a fake gpg that runs sleep as a
// child instead of exec-ing it, the way a wrapper script around the real // 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 // gpg might. Killing the fake gpg leaves sleep holding its stdout and
// stderr open; the call must still return once ctx ends instead of waiting // stderr open; the call must still return shortly after the deadline
// for sleep to exit. The fake gpg writes the process ID of sleep to a named // instead of waiting for sleep to exit. The fake gpg writes the process ID
// pipe; the test ends ctx only after reading it, so sleep is running by // of sleep to a file so that the test can kill it before returning.
// then, and kills sleep before returning. func TestGPGTimeoutWhenChildHoldsOutput(t *testing.T) {
func TestGPGCancelWhenChildHoldsOutput(t *testing.T) { pidFile := filepath.Join(t.TempDir(), "sleep.pid")
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, t.Setenv("PATH", fakeGPGPath(t,
"#!/bin/sh\nsleep 60 &\necho $! >'"+pidPipe+"'\nwait\n")) "#!/bin/sh\nsleep 3 &\necho $! >'"+pidFile+"'\nwait\n"))
t.Cleanup(func() {
ctx, cancel := context.WithCancel(context.Background()) pid, err := os.ReadFile(pidFile) //nolint:gosec // G304: path inside t.TempDir()
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) require.NoError(t, err)
n, err := strconv.Atoi(strings.TrimSpace(string(pid))) n, err := strconv.Atoi(strings.TrimSpace(string(pid)))
@@ -458,19 +444,17 @@ func TestGPGCancelWhenChildHoldsOutput(t *testing.T) {
sleep, err := os.FindProcess(n) sleep, err := os.FindProcess(n)
require.NoError(t, err) require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, sleep.Kill()) }) require.NoError(t, sleep.Kill())
})
cancel() ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
// The call should return about gpgWaitDelay (one second) after the start := time.Now()
// cancel. 10 s is far above that and well under the 30 s test timeout, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
// which would abort the whole package before the cleanup kills sleep. require.ErrorIs(t, err, context.DeadlineExceeded)
select { assert.Less(t, time.Since(start), 3*time.Second,
case err := <-signErr: "the call waited for the child holding gpg's output to exit")
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 // TestBuildPassesContextToSigning checks that a caller can cancel the gpg
+29 -2
View File
@@ -304,19 +304,46 @@ func TestScannerEnumerateFS(t *testing.T) {
func TestSendEnumerateStatusNonBlocking(t *testing.T) { func TestSendEnumerateStatusNonBlocking(t *testing.T) {
t.Parallel() t.Parallel()
// Nobody receives, so a blocking send would hang the test into its timeout. // Channel with no buffer - send should not block
ch := make(chan EnumerateStatus) ch := make(chan EnumerateStatus)
// This should not block
done := make(chan bool)
go func() {
sendEnumerateStatus(ch, EnumerateStatus{FilesFound: 1}) 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) { func TestSendScanStatusNonBlocking(t *testing.T) {
t.Parallel() t.Parallel()
// Nobody receives, so a blocking send would hang the test into its timeout. // Channel with no buffer - send should not block
ch := make(chan ScanStatus) ch := make(chan ScanStatus)
done := make(chan bool)
go func() {
sendScanStatus(ch, ScanStatus{ScannedFiles: 1}) 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) { func TestSendStatusNilChannel(t *testing.T) {