encoding/pem: properly decode strange PEM data

When the passed byte slice has leading garbage, properly handle ignoring
it and continuing to parse the slice until we find a valid block (or
nothing).

Change-Id: I07e937d9c754fd71b028b99450b48f57b4464457
Reviewed-on: https://go-review.googlesource.com/c/go/+/712140
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/encoding/pem/pem.go

@@ -91,7 +91,12 @@ func Decode(data []byte) (p *Block, rest []byte) {
 	// the byte array, we'll accept the start string without it.
 	rest = data
 
+	endTrailerIndex := 0
 	for {
+		// If we've already tried parsing a block, skip past the END we already
+		// saw.
+		rest = rest[endTrailerIndex:]
+
 		// Find the first END line, and then find the last BEGIN line before
 		// the end line. This lets us skip any repeated BEGIN lines that don't
 		// have a matching END.
@@ -99,10 +104,10 @@ func Decode(data []byte) (p *Block, rest []byte) {
 		if endIndex < 0 {
 			return nil, data
 		}
-		endTrailerIndex := endIndex + len(pemEnd)
+		endTrailerIndex = endIndex + len(pemEnd)
 		beginIndex := bytes.LastIndex(rest[:endIndex], pemStart[1:])
-		if beginIndex < 0 || beginIndex > 0 && rest[beginIndex-1] != '\n' {
-			return nil, data
+		if beginIndex < 0 || (beginIndex > 0 && rest[beginIndex-1] != '\n') {
+			continue
 		}
 		rest = rest[beginIndex+len(pemStart)-1:]
 		endIndex -= beginIndex + len(pemStart) - 1

src/encoding/pem/pem_test.go

@@ -639,3 +639,100 @@ func TestBadEncode(t *testing.T) {
 }
 
 func testingKey(s string) string { return strings.ReplaceAll(s, "TESTING KEY", "PRIVATE KEY") }
+
+func TestDecodeStrangeCases(t *testing.T) {
+	sentinelType := "TEST BLOCK"
+	sentinelBytes := []byte("hello")
+	for _, tc := range []struct {
+		name string
+		pem  string
+	}{
+		{
+			name: "invalid section (not base64)",
+			pem: `-----BEGIN COMMENT-----
+foo foo foo
+-----END COMMENT-----
+-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----`,
+		},
+		{
+			name: "leading garbage on block",
+			pem: `foo foo foo-----BEGIN CERTIFICATE-----
+MCowBQYDK2VwAyEApVjJeLW5MoP6uR3+OeITokM+rBDng6dgl1vvhcy+wws=
+-----END PUBLIC KEY-----
+-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----`,
+		},
+		{
+			name: "leading garbage",
+			pem: `foo foo foo
+-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----`,
+		},
+		{
+			name: "leading partial block",
+			pem: `foo foo foo
+-----END COMMENT-----
+-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----`,
+		},
+		{
+			name: "multiple BEGIN",
+			pem: `-----BEGIN TEST BLOCK-----
+-----BEGIN TEST BLOCK-----
+-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----`,
+		},
+		{
+			name: "multiple END",
+			pem: `-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----
+-----END TEST BLOCK-----
+-----END TEST BLOCK-----`,
+		},
+		{
+			name: "leading malformed BEGIN",
+			pem: `-----BEGIN PUBLIC KEY
+aGVsbG8=
+-----END PUBLIC KEY-----
+-----BEGIN TEST BLOCK-----
+aGVsbG8=
+-----END TEST BLOCK-----`,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			block, _ := Decode([]byte(tc.pem))
+			if block == nil {
+				t.Fatal("expected valid block")
+			}
+			if block.Type != sentinelType {
+				t.Fatalf("unexpected block returned, got type %q, want type %q", block.Type, sentinelType)
+			}
+			if !bytes.Equal(block.Bytes, sentinelBytes) {
+				t.Fatalf("unexpected block content, got %x, want %x", block.Bytes, sentinelBytes)
+			}
+		})
+	}
+}
+
+func TestJustEnd(t *testing.T) {
+	pemData := `
+-----END PUBLIC KEY-----`
+
+	block, _ := Decode([]byte(pemData))
+	if block != nil {
+		t.Fatal("unexpected block")
+	}
+}
+
+func FuzzDecode(f *testing.F) {
+	f.Fuzz(func(t *testing.T, data []byte) {
+		Decode(data)
+	})
+}