From f0e8d0fe7c756a25e855f95392ed617fcaa8357a Mon Sep 17 00:00:00 2001
From: Eric Chiang <ericchiang@google.com>
Date: Wed, 9 Oct 2019 08:39:15 -0700
Subject: [PATCH] attest: fix another unbounded memory allocation

@brandonweeks detected another case of the "make([]T, untrustedValue)"
pattern, which would allow an attacker to cause the parser to allocate
an unbounded amount of memory.

Fix this by reading one algorithm at a time instead of pre-allocating a
slice of algorithms.
---
 attest/eventlog.go      |  14 +++---
 attest/eventlog_test.go | 108 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+), 7 deletions(-)

diff --git a/attest/eventlog.go b/attest/eventlog.go
index a3c4d7d..b041e27 100644
--- a/attest/eventlog.go
+++ b/attest/eventlog.go
@@ -444,9 +444,13 @@ func parseSpecIDEvent(b []byte) (*specIDEvent, error) {
 	// TODO(ericchiang): Check errata? Or do we expect that to change in ways
 	// we're okay with?
 
-	algs := make([]specAlgSize, header.NumAlgs)
-	if err := binary.Read(r, binary.LittleEndian, &algs); err != nil {
-		return nil, fmt.Errorf("reading algorithms: %v", err)
+	specAlg := specAlgSize{}
+	e := specIDEvent{}
+	for i := 0; i < int(header.NumAlgs); i++ {
+		if err := binary.Read(r, binary.LittleEndian, &specAlg); err != nil {
+			return nil, fmt.Errorf("reading algorithm: %v", err)
+		}
+		e.algs = append(e.algs, specAlg.ID)
 	}
 
 	var vendorInfoSize uint8
@@ -456,10 +460,6 @@ func parseSpecIDEvent(b []byte) (*specIDEvent, error) {
 	if r.Len() != int(vendorInfoSize) {
 		return nil, fmt.Errorf("reading vendor info, expected %d remaining bytes, got %d", vendorInfoSize, r.Len())
 	}
-	var e specIDEvent
-	for _, alg := range algs {
-		e.algs = append(e.algs, alg.ID)
-	}
 	return &e, nil
 }
 
diff --git a/attest/eventlog_test.go b/attest/eventlog_test.go
index a6b4631..169aa60 100644
--- a/attest/eventlog_test.go
+++ b/attest/eventlog_test.go
@@ -148,3 +148,111 @@ func TestParseEventLogEventSizeTooLarge(t *testing.T) {
 		t.Fatalf("expected parsing invalid event log to fail")
 	}
 }
+
+func TestParseSpecIDEvent(t *testing.T) {
+	tests := []struct {
+		name    string
+		data    []byte
+		want    []uint16
+		wantErr bool
+	}{
+		{
+			name: "sha1",
+			data: append(
+				[]byte("Spec ID Event03"), 0x0,
+				0x0, 0x0, 0x0, 0x0, // platform class
+				0x0,                // verison minor
+				0x2,                // version major
+				0x0,                // errata
+				0x8,                // uintn size
+				0x1, 0x0, 0x0, 0x0, // num algs
+				0x04, 0x0, // SHA1
+				0x14, 0x0, // size
+				0x2, // vendor info size
+				0x0, 0x0,
+			),
+			want: []uint16{0x0004},
+		},
+		{
+			name: "sha1_and_sha256",
+			data: append(
+				[]byte("Spec ID Event03"), 0x0,
+				0x0, 0x0, 0x0, 0x0, // platform class
+				0x0,                // verison minor
+				0x2,                // version major
+				0x0,                // errata
+				0x8,                // uintn size
+				0x2, 0x0, 0x0, 0x0, // num algs
+				0x04, 0x0, // SHA1
+				0x14, 0x0, // size
+				0x0B, 0x0, // SHA256
+				0x20, 0x0, // size
+				0x2, // vendor info size
+				0x0, 0x0,
+			),
+			want: []uint16{0x0004, 0x000B},
+		},
+		{
+			name: "invalid_version",
+			data: append(
+				[]byte("Spec ID Event03"), 0x0,
+				0x0, 0x0, 0x0, 0x0, // platform class
+				0x2,                // verison minor
+				0x1,                // version major
+				0x0,                // errata
+				0x8,                // uintn size
+				0x2, 0x0, 0x0, 0x0, // num algs
+				0x04, 0x0, // SHA1
+				0x14, 0x0, // size
+				0x0B, 0x0, // SHA256
+				0x20, 0x0, // size
+				0x2, // vendor info size
+				0x0, 0x0,
+			),
+			wantErr: true,
+		},
+		{
+			name: "malicious_number_of_algs",
+			data: append(
+				[]byte("Spec ID Event03"), 0x0,
+				0x0, 0x0, 0x0, 0x0, // platform class
+				0x0,                    // verison minor
+				0x2,                    // version major
+				0x0,                    // errata
+				0x8,                    // uintn size
+				0xff, 0xff, 0xff, 0xff, // num algs
+				0x04, 0x0, // SHA1
+				0x14, 0x0, // size
+				0x2, // vendor info size
+				0x0, 0x0,
+			),
+			wantErr: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			spec, err := parseSpecIDEvent(test.data)
+			if (err != nil) != test.wantErr {
+				t.Fatalf("parsing spec, wantErr=%t, got=%v", test.wantErr, err)
+			}
+			if err != nil {
+				return
+			}
+			algsEq := func(got, want []uint16) bool {
+				if len(got) != len(want) {
+					return false
+				}
+				for i, alg := range got {
+					if want[i] != alg {
+						return false
+					}
+				}
+				return true
+			}
+
+			if !algsEq(test.want, spec.algs) {
+				t.Errorf("algorithms, got=%x, want=%x", spec.algs, test.want)
+			}
+		})
+	}
+}