CVE-2022-2584: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The dag-pb codec can panic when decoding invalid blocks.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 7.5
Published: 2022-12-27T22:15Z
Repository: https://github.com/ipld/go-codec-dagpb
Language: Go
Files changed: compat_test.go, unmarshal.go
Commit date: 2022-03-16 20:11:00 +1100
Commit message: fix: use protowire for Links bytes decoding

Diff: diff --git a/compat_test.go b/compat_test.go
index 3cc4307..fa3dedf 100644
--- a/compat_test.go
+++ b/compat_test.go
@@ -30,10 +30,11 @@ type testCase struct {
 	node          *pbNode
 	expectedBytes string
 	expectedForm  string
 	encodeError   string
 	decodeError   string
+	testEncode    bool
 }
 
 var testCases = []testCase{
 	{
 		name:          "empty",
@@ -62,10 +63,17 @@ var testCases = []testCase{
 	"Data": "0001020304",
 	"Links": []
 }`,
 		encodeError: "missing required fields: Links",
 	},
+	{
+		name:          "Data some, short",
+		node:          &pbNode{data: dataSome},
+		expectedBytes: "0a0500010203",
+		decodeError:   "unexpected EOF",
+		encodeError:   "missing required fields: Links",
+	},
 	{
 		name:          "Links zero",
 		node:          &pbNode{links: []pbLink{}},
 		expectedBytes: "",
 		expectedForm: `{
@@ -111,10 +119,16 @@ var testCases = []testCase{
 		}
 	]
 }`,
 	},
 	{
+		name:          "Links Hash some, short",
+		node:          &pbNode{links: []pbLink{{hash: acid}}},
+		expectedBytes: "120b0a090155000500010203",
+		decodeError:   "unexpected EOF",
+		testEncode:    false,
+	}, {
 		name:          "Links Name zero",
 		node:          &pbNode{links: []pbLink{{name: zeroName, hasName: true}}},
 		expectedBytes: "12021200",
 		encodeError:   "missing required fields: Hash",
 		decodeError:   "expected CID",
@@ -209,28 +223,30 @@ func verifyRoundTrip(t *testing.T, tc testCase) {
 
 	if tc.node != nil {
 		node := buildNode(*tc.node)
 		actualBytes, err = nodeToString(t, node)
 
-		if tc.encodeError != "" {
-			if err != nil {
-				if !strings.Contains(err.Error(), tc.encodeError) {
-					t.Fatalf("got unexpeced encode error: [%v] (expected [%v])", err.Error(), tc.encodeError)
+		if tc.testEncode {
+			if tc.encodeError != "" {
+				if err != nil {
+					if !strings.Contains(err.Error(), tc.encodeError) {
+						t.Fatalf("got unexpeced encode error: [%v] (expected [%v])", err.Error(), tc.encodeError)
+					}
+				} else {
+					t.Fatalf("did not get expected encode error: %v", tc.encodeError)
 				}
 			} else {
-				t.Fatalf("did not get expected encode error: %v", tc.encodeError)
-			}
-		} else {
-			if err != nil {
-				t.Fatal(err)
-			} else {
-				if actualBytes != tc.expectedBytes {
-					t.Logf(
-						"Expected bytes: [%v]\nGot: [%v]\n",
-						tc.expectedBytes,
-						actualBytes)
-					t.Error("Did not match")
+				if err != nil {
+					t.Fatal(err)
+				} else {
+					if actualBytes != tc.expectedBytes {
+						t.Logf(
+							"Expected bytes: [%v]\nGot: [%v]\n",
+							tc.expectedBytes,
+							actualBytes)
+						t.Error("Did not match")
+					}
 				}
 			}
 		}
 	}
 
diff --git a/unmarshal.go b/unmarshal.go
index 4049859..58e245b 100644
--- a/unmarshal.go
+++ b/unmarshal.go
@@ -96,11 +96,11 @@ func DecodeBytes(na ipld.NodeAssembler, src []byte) error {
 				return err
 			}
 			haveData = true
 
 		case 2:
-			bytesLen, n := protowire.ConsumeVarint(remaining)
+			chunk, n := protowire.ConsumeBytes(remaining)
 			if n < 0 {
 				return protowire.ParseError(n)
 			}
 			remaining = remaining[n:]
 
@@ -121,14 +121,13 @@ func DecodeBytes(na ipld.NodeAssembler, src []byte) error {
 
 			curLink, err := links.AssembleValue().BeginMap(3)
 			if err != nil {
 				return err
 			}
-			if err := unmarshalLink(remaining[:bytesLen], curLink); err != nil {
+			if err := unmarshalLink(chunk, curLink); err != nil {
 				return err
 			}
-			remaining = remaining[bytesLen:]
 			if err := curLink.Finish(); err != nil {
 				return err
 			}
 			haveLinks = true
 

Vulnerable code: 		if tc.encodeError != "" {
			if err != nil {
				if !strings.Contains(err.Error(), tc.encodeError) {
					t.Fatalf("got unexpeced encode error: [%v] (expected [%v])", err.Error(), tc.encodeError)
				t.Fatalf("did not get expected encode error: %v", tc.encodeError)
			}
		} else {
			if err != nil {
				t.Fatal(err)
			} else {
				if actualBytes != tc.expectedBytes {
					t.Logf(
						"Expected bytes: [%v]\nGot: [%v]\n",
						tc.expectedBytes,
						actualBytes)
					t.Error("Did not match")
			bytesLen, n := protowire.ConsumeVarint(remaining)
			if err := unmarshalLink(remaining[:bytesLen], curLink); err != nil {
			remaining = remaining[bytesLen:]
Fixed code: 	testEncode    bool
	{
		name:          "Data some, short",
		node:          &pbNode{data: dataSome},
		expectedBytes: "0a0500010203",
		decodeError:   "unexpected EOF",
		encodeError:   "missing required fields: Links",
	},
		name:          "Links Hash some, short",
		node:          &pbNode{links: []pbLink{{hash: acid}}},
		expectedBytes: "120b0a090155000500010203",
		decodeError:   "unexpected EOF",
		testEncode:    false,
	}, {
		if tc.testEncode {
			if tc.encodeError != "" {
				if err != nil {
					if !strings.Contains(err.Error(), tc.encodeError) {
						t.Fatalf("got unexpeced encode error: [%v] (expected [%v])", err.Error(), tc.encodeError)
					}
				} else {
					t.Fatalf("did not get expected encode error: %v", tc.encodeError)
				if err != nil {
					t.Fatal(err)
				} else {
					if actualBytes != tc.expectedBytes {
						t.Logf(
							"Expected bytes: [%v]\nGot: [%v]\n",
							tc.expectedBytes,
							actualBytes)
						t.Error("Did not match")
					}
			chunk, n := protowire.ConsumeBytes(remaining)
			if err := unmarshalLink(chunk, curLink); err != nil {