diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe.go b/syft/pkg/cataloger/internal/dotnet/pe/pe.go index c389d55bd..9ffee6648 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/pe.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe.go @@ -607,6 +607,11 @@ func parseVersionResourceSection(reader *bytes.Reader, fields map[string]string) var sfiHeader peStringFileInfo if szKey, err := readIntoStructAndSzKey(reader, &sfiHeader, &offset); err != nil { + if isTruncated(err) { + // a well-formed version resource whose last child is VarFileInfo ends right here, so stop + // and let the FileVersion fallback below still run + break + } return fmt.Errorf("error reading PE string file info header: %v", err) } else if szKey != "StringFileInfo" { // we only care about extracting strings from any string tables, skip this @@ -619,31 +624,14 @@ func parseVersionResourceSection(reader *bytes.Reader, fields map[string]string) // note: the szKey for the prStringTable is the language var stHeader peStringTable if _, err := readIntoStructAndSzKey(reader, &stHeader, &offset, &stOffset); err != nil { + if isTruncated(err) { + break + } return fmt.Errorf("error reading PE string table header: %v", err) } - for stOffset < int(stHeader.Length) { - var stringHeader peString - if err := readIntoStruct(reader, &stringHeader, &offset, &stOffset); err != nil { - break - } - - key := readUTF16(reader, &offset, &stOffset) - - if err := alignAndSeek(reader, &offset, &stOffset); err != nil { - return fmt.Errorf("error aligning to next PE string table value: %w", err) - } - - var value string - if stringHeader.ValueLength > 0 { - value = readUTF16(reader, &offset, &stOffset) - } - - fields[key] = value - - if err := alignAndSeek(reader, &offset, &stOffset); err != nil { - return fmt.Errorf("error aligning to next PE string table key: %w", err) - } + if err := parseStringTable(reader, int(stHeader.Length), &offset, &stOffset, fields); err != nil { + return err } } @@ -657,6 +645,40 @@ func parseVersionResourceSection(reader *bytes.Reader, fields map[string]string) return nil } +// parseStringTable reads the key/value pairs of a single string table into fields. length is what the +// string table header claims it holds, and stOffset tracks how much of that has actually been consumed. +func parseStringTable(reader *bytes.Reader, length int, offset, stOffset *int, fields map[string]string) error { + for *stOffset < length { + var stringHeader peString + if err := readIntoStruct(reader, &stringHeader, offset, stOffset); err != nil { + if isTruncated(err) { + // the table claims more content than the resource carries; stop rather than re-reading a + // reader that is not advancing + break + } + return fmt.Errorf("error reading PE string table entry: %w", err) + } + + key := readUTF16(reader, offset, stOffset) + + if err := alignAndSeek(reader, offset, stOffset); err != nil { + return fmt.Errorf("error aligning to next PE string table value: %w", err) + } + + var value string + if stringHeader.ValueLength > 0 { + value = readUTF16(reader, offset, stOffset) + } + + fields[key] = value + + if err := alignAndSeek(reader, offset, stOffset); err != nil { + return fmt.Errorf("error aligning to next PE string table key: %w", err) + } + } + return nil +} + // readIntoStructAndSzKey reads a struct from the reader and updates the offsets if provided, returning the szKey value. // This is only useful in the context of the resource directory parsing in narrow cases (this is invalid to use outside of that context). func readIntoStructAndSzKey[T any](reader *bytes.Reader, data *T, offsets ...*int) (string, error) { @@ -666,12 +688,19 @@ func readIntoStructAndSzKey[T any](reader *bytes.Reader, data *T, offsets ...*in return readUTF16(reader, offsets...), nil } +// isTruncated reports whether err means the resource simply ran out of bytes. binary.Read gives io.EOF when +// nothing was left and io.ErrUnexpectedEOF when a struct was cut in half; both say the same thing about a +// version resource, and neither should cost us the fields already collected. +func isTruncated(err error) bool { + return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) +} + // readIntoStruct reads a struct from the reader and updates the offsets if provided. +// +// note: a truncated read must stay an error. Callers advance their loop counters by the offsets updated +// below, so reporting a zeroed struct as a successful read leaves length-driven loops spinning forever. func readIntoStruct[T any](reader io.Reader, data *T, offsets ...*int) error { if err := binary.Read(reader, binary.LittleEndian, data); err != nil { - if errors.Is(err, io.EOF) { - return nil - } return err } diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go b/syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go new file mode 100644 index 000000000..1b302ef3f --- /dev/null +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go @@ -0,0 +1,149 @@ +package pe + +import ( + "bytes" + "encoding/binary" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// buildVersionResource returns a well-formed VS_VERSION_INFO blob whose last child is a VarFileInfo +// block, which is the layout real toolchains emit. It ends exactly on a struct boundary. When +// withFileVersionString is set, a StringFileInfo table supplies FileVersion directly. +func buildVersionResource(withFileVersionString bool) []byte { + buf := new(bytes.Buffer) + le := binary.LittleEndian + + putHeader := func(length, valueLength, typ uint16) { + _ = binary.Write(buf, le, [3]uint16{length, valueLength, typ}) + } + putUTF16 := func(s string) { + for _, r := range s { + _ = binary.Write(buf, le, uint16(r)) + } + _ = binary.Write(buf, le, uint16(0)) + } + pad := func() { + for buf.Len()%4 != 0 { + buf.WriteByte(0) + } + } + + putHeader(0, 52, 0) + putUTF16("VS_VERSION_INFO") + pad() + + // peVsFixedFileInfo, with FileVersionMS/LS encoding 1.2.3.4 + ffi := make([]byte, 52) + le.PutUint32(ffi[0:], 0xFEEF04BD) // signature + le.PutUint32(ffi[4:], 0x00010000) // strucVersion + le.PutUint32(ffi[8:], 0x00010002) // FileVersionMS -> 1.2 + le.PutUint32(ffi[12:], 0x00030004) // FileVersionLS -> 3.4 + buf.Write(ffi) + + if withFileVersionString { + pad() + putHeader(0, 0, 1) + putUTF16("StringFileInfo") + pad() + putHeader(38, 0, 1) + putUTF16("040904b0") + pad() + putHeader(0, 8, 1) + putUTF16("FileVersion") + pad() + putUTF16("9.9.9.9") + } + + // the final child: VarFileInfo -> Var("Translation") with a 4 byte value + pad() + putHeader(0, 0, 1) + putUTF16("VarFileInfo") + pad() + putHeader(0, 4, 0) + putUTF16("Translation") + pad() + _ = binary.Write(buf, le, uint32(0x04b00409)) + + return buf.Bytes() +} + +func TestParseVersionResourceSection_FileVersionFallback(t *testing.T) { + // a resource ending on a struct boundary is normal termination, not a parse failure. Treating it as + // an error skips the VS_FIXEDFILEINFO fallback below, which is the only source of a version for + // binaries that carry no FileVersion string entry. + tests := []struct { + name string + withFileVersionString bool + want string + }{ + { + name: "derived from fixed file info when no string entry exists", + want: "1.2.3.4", + }, + { + name: "string entry wins when present", + withFileVersionString: true, + want: "9.9.9.9", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fields := map[string]string{} + require.NoError(t, parseVersionResourceSection(bytes.NewReader(buildVersionResource(tt.withFileVersionString)), fields)) + assert.Equal(t, tt.want, fields["FileVersion"]) + }) + } +} + +// buildTruncatedStringTable returns version resource bytes that end exactly on a struct boundary while +// the string table header still claims 0xFFFF bytes remain. Landing precisely at EOF is the case that +// matters: one to five trailing bytes yield io.ErrUnexpectedEOF, which was always handled. +func buildTruncatedStringTable() []byte { + buf := new(bytes.Buffer) + putHeader := func() { + _ = binary.Write(buf, binary.LittleEndian, [3]uint16{}) // Length, ValueLength, Type + } + putUTF16 := func(s string) { + for _, r := range s { + _ = binary.Write(buf, binary.LittleEndian, uint16(r)) + } + _ = binary.Write(buf, binary.LittleEndian, uint16(0)) // null terminator + } + + putHeader() + putUTF16("VS_VERSION_INFO") // offset 38 + buf.Write([]byte{0, 0}) // pad to the DWORD boundary at 40 + buf.Write(make([]byte, 52)) // peVsFixedFileInfo -> 92 + + putHeader() + putUTF16("StringFileInfo") // -> 128 + + // the string table header claims far more content than the file carries + _ = binary.Write(buf, binary.LittleEndian, [3]uint16{0xFFFF, 0, 0}) + putUTF16("040904b0") // -> 152, then EOF + + return buf.Bytes() +} + +func TestParseVersionResourceSection_TruncatedStringTableTerminates(t *testing.T) { + data := buildTruncatedStringTable() + + done := make(chan error, 1) + go func() { + done <- parseVersionResourceSection(bytes.NewReader(data), map[string]string{}) + }() + + select { + case <-done: + case <-time.After(5 * time.Second): + // a reader parked at EOF used to be reported as a successful read of a zeroed struct, so the + // string table loop consumed nothing, advanced nothing, and never reached its length bound + require.FailNow(t, "parseVersionResourceSection did not terminate", + "a %d-byte resource blob must not spin forever", len(data)) + } +}