Reject manifests whose file entries decode far larger than their bytes (closes #123)
check / check (push) Failing after 2s
check / check (push) Failing after 2s
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
This commit was merged in pull request #128.
This commit is contained in:
+91
-2
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/google/uuid"
|
||||
"github.com/klauspost/compress/zstd"
|
||||
"github.com/spf13/afero"
|
||||
"google.golang.org/protobuf/encoding/protowire"
|
||||
"google.golang.org/protobuf/proto"
|
||||
"sneak.berlin/go/mfer/internal/bork"
|
||||
"sneak.berlin/go/mfer/internal/log"
|
||||
@@ -27,6 +28,8 @@ var (
|
||||
errUUIDMismatch = errors.New("outer and inner UUID mismatch")
|
||||
errInvalidFileFormat = errors.New("invalid file format")
|
||||
errInvalidManifestPath = errors.New("manifest contains invalid path")
|
||||
errDecodedTooLarge = errors.New(
|
||||
"manifest would take too much memory to decode")
|
||||
)
|
||||
|
||||
// validateUUID checks that the byte slice is a valid UUID (16 bytes, parseable).
|
||||
@@ -154,6 +157,85 @@ func (m *manifest) decompressInner() ([]byte, error) {
|
||||
return dat, nil
|
||||
}
|
||||
|
||||
// checkDecodedSize refuses an encoded inner message whose file entries,
|
||||
// hashes, timestamps and MIME types would take more than maxDecodedGrowth
|
||||
// times its size to decode. Decoding sets aside a fixed amount for each,
|
||||
// however short its encoding, so a message of empty ones would take about
|
||||
// 50 times its size.
|
||||
func checkDecodedSize(inner []byte) error {
|
||||
limit := maxDecodedGrowth * int64(len(inner))
|
||||
|
||||
var decoded int64
|
||||
|
||||
add := func(size int64) error {
|
||||
decoded += size
|
||||
if decoded > limit {
|
||||
return errDecodedTooLarge
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
return forEachBytesField(inner, func(num protowire.Number, entry []byte) error {
|
||||
if num != filesFieldNumber {
|
||||
return nil
|
||||
}
|
||||
|
||||
err := add(decodedFileEntrySize)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return forEachBytesField(entry, func(num protowire.Number, _ []byte) error {
|
||||
if num == hashesFieldNumber {
|
||||
return add(decodedHashSize)
|
||||
}
|
||||
|
||||
if num == mtimeFieldNumber || num == ctimeFieldNumber {
|
||||
return add(decodedTimestampSize)
|
||||
}
|
||||
|
||||
if num == mimeTypeFieldNumber {
|
||||
return add(decodedMIMETypeSize)
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// forEachBytesField calls fn with the number and value of each
|
||||
// length-delimited field in the encoded message msg, and fails if msg is
|
||||
// malformed.
|
||||
func forEachBytesField(
|
||||
msg []byte, fn func(num protowire.Number, value []byte) error,
|
||||
) error {
|
||||
for len(msg) > 0 {
|
||||
num, wireType, tagLen := protowire.ConsumeTag(msg)
|
||||
if tagLen < 0 {
|
||||
return protowire.ParseError(tagLen)
|
||||
}
|
||||
|
||||
valueLen := protowire.ConsumeFieldValue(num, wireType, msg[tagLen:])
|
||||
if valueLen < 0 {
|
||||
return protowire.ParseError(valueLen)
|
||||
}
|
||||
|
||||
if wireType == protowire.BytesType {
|
||||
value, _ := protowire.ConsumeBytes(msg[tagLen:])
|
||||
|
||||
err := fn(num, value)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
msg = msg[tagLen+valueLen:]
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *manifest) deserializeInner() error {
|
||||
err := m.validateOuterHeader()
|
||||
if err != nil {
|
||||
@@ -177,10 +259,16 @@ func (m *manifest) deserializeInner() error {
|
||||
return bork.ErrFileTruncated
|
||||
}
|
||||
|
||||
err = checkDecodedSize(dat)
|
||||
if err != nil {
|
||||
return fmt.Errorf("deserialize: unmarshal inner: %w", err)
|
||||
}
|
||||
|
||||
// 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)
|
||||
}
|
||||
@@ -249,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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user