Fix bounds check in amd64 decoder

pierrec · Aug 28, 2021 · 433be85 · 433be85
1 parent fb71485
commit 433be85
Show file tree

Hide file tree

Showing 6 changed files with 9 additions and 10 deletions.
diff --git a/fuzz/uncompress/corpus/0a5174c42818e5ce11cec8b830842a642b5d9c2a-8 b/fuzz/uncompress/corpus/0a5174c42818e5ce11cec8b830842a642b5d9c2a-8
diff --git a/fuzz/uncompress/corpus/120bbfdceaa46d29590c2b8e13d32b24bc12bd92-13 b/fuzz/uncompress/corpus/120bbfdceaa46d29590c2b8e13d32b24bc12bd92-13
diff --git a/fuzz/uncompress/corpus/edddcb660ba75b99edb3e12bcdb3fe0fcc55ba81 b/fuzz/uncompress/corpus/edddcb660ba75b99edb3e12bcdb3fe0fcc55ba81
diff --git a/fuzz/uncompress/corpus/fb1320de40de93ece69de3f68bea1b6bafa0fdfe-7 b/fuzz/uncompress/corpus/fb1320de40de93ece69de3f68bea1b6bafa0fdfe-7
diff --git a/internal/lz4block/decode_amd64.s b/internal/lz4block/decode_amd64.s
@@ -18,7 +18,7 @@
 // R12 short output end
 // R13 short input end
 // R14 &dict
-// R15 &dict + len(dict)
+// R15 len(dict)
 
 // func decodeBlock(dst, src, dict []byte) int
 TEXT ·decodeBlock(SB), NOSPLIT, $48-80
@@ -35,7 +35,6 @@ TEXT ·decodeBlock(SB), NOSPLIT, $48-80
 
 	MOVQ dict_base+48(FP), R14
 	MOVQ dict_len+56(FP), R15
-	ADDQ R14, R15
 
 	// shortcut ends
 	// short output end
@@ -210,7 +209,6 @@ memmove_lit:
 	ADDQ src_len+32(FP), R9
 	MOVQ dict_base+48(FP), R14
 	MOVQ dict_len+56(FP), R15
-	ADDQ R14, R15
 	MOVQ R8, R12
 	SUBQ $32, R12
 	MOVQ R9, R13
@@ -332,14 +330,12 @@ copy_match_from_dict:
 	MOVQ R11, AX
 	SUBQ BX, AX
 
-	// BX = &dict_end - dict_bytes_available
+	// BX = len(dict) - dict_bytes_available
 	MOVQ R15, BX
 	SUBQ AX, BX
+	JS err_short_dict
 
-	// check BX is within dict
-	// if BX < &dict
-	CMPQ BX, R14
-	JB err_short_dict
+	ADDQ R14, BX
 
 	// if match_len > dict_bytes_available, match fits entirely within external dictionary : just copy
 	CMPQ CX, AX
@@ -376,7 +372,6 @@ copy_match_from_dict:
 	ADDQ src_len+32(FP), R9
 	MOVQ dict_base+48(FP), R14
 	MOVQ dict_len+56(FP), R15
-	ADDQ R14, R15
 	MOVQ R8, R12
 	SUBQ $32, R12
 	MOVQ R9, R13
@@ -428,7 +423,6 @@ memmove_match:
 	SUBQ $16, R13
 	MOVQ dict_base+48(FP), R14
 	MOVQ dict_len+56(FP), R15
-	ADDQ R14, R15
 
 	JMP loop
 

diff --git a/internal/lz4block/decode_test.go b/internal/lz4block/decode_test.go
@@ -159,6 +159,11 @@ func TestDecodeBlockInvalid(t *testing.T) {
 			"\x1b0\x01\x00000000000000",
 			len("\x1b0\x01\x00000000000000"),
 		},
+		{
+			"bounds-crasher", // Triggered a broken bounds check in amd64 decoder.
+			"\x000000",
+			10,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			dst := make([]byte, test.size+8)