Fix "crc32sseAll" out of bounds read.

When length was 7, an extra byte would be read from the input. In rare cases that would hit a page boundary when flushing and crash.

The test has been extended to increase the chance of detecting a situation like this.

Fixes #20

README.md

@@ -12,6 +12,7 @@ It offers slightly better compression at lower compression settings, and up to 3
 
 # changelog
 
+* Dec 8 2015: Fixed rare [one byte out-of bounds read](https://github.com/klauspost/compress/issues/20). Please update!
 * Nov 20 2015: Small optimization to bit writer on 64 bit systems.
 * Nov 17 2015: Fixed out-of-bound errors if the underlying Writer returned an error. See [#15](https://github.com/klauspost/compress/issues/15).
 * Nov 12 2015: Added [io.WriterTo](https://golang.org/pkg/io/#WriterTo) support to gzip/inflate.

flate/crc32_amd64.s

@@ -10,29 +10,24 @@ TEXT ·crc32sse(SB),7, $0
     BYTE $0xF2; BYTE $0x41; BYTE $0x0f
     BYTE $0x38; BYTE $0xf1; BYTE $0x1a
 
-
-    // MOVL    (R10), AX
-    // CRC32   EAX, EBX
-    //BYTE $0xF2; BYTE $0x0f;
-    //BYTE $0x38; BYTE $0xf1; BYTE $0xd8
-
     MOVL    BX, ret+24(FP)
     RET
 
 // func crc32sseAll(a []byte, dst []hash)
 TEXT ·crc32sseAll(SB), 7, $0
-    MOVQ    a+0(FP), R8
-    MOVQ    a_len+8(FP), R10
-    MOVQ    dst+24(FP), R9
-    MOVQ    $0, AX
-    SUBQ    $3, R10
-    JZ      end
+    MOVQ    a+0(FP), R8     // R8: src
+    MOVQ    a_len+8(FP), R10  // input length
+    MOVQ    dst+24(FP), R9  // R9: dst
+    SUBQ    $4, R10
     JS      end
+    JZ      one_crc
     MOVQ    R10, R13
     SHRQ    $2, R10  // len/4
     ANDQ    $3, R13  // len&3
-    TESTQ   R10,R10
-    JZ      remain_crc
+    XORQ    BX, BX    
+    ADDQ    $1, R13
+    TESTQ   R10, R10
+    JZ      rem_loop
 
 crc_loop:
     MOVQ    (R8), R11
@@ -70,21 +65,18 @@ crc_loop:
 
     ADDQ    $16, R9
     ADDQ    $4, R8
+    XORQ    BX, BX
     SUBQ    $1, R10
     JNZ     crc_loop
 
-remain_crc:
-    XORQ    BX, BX
-    TESTQ    R13, R13
-    JZ      end
 rem_loop:
     MOVL    (R8), AX
 
     // CRC32   EAX, EBX
     BYTE $0xF2; BYTE $0x0f;
     BYTE $0x38; BYTE $0xf1; BYTE $0xd8
 
-    MOVL    BX,(R9)
+    MOVL    BX, (R9)
     ADDQ    $4, R9
     ADDQ    $1, R8
     XORQ    BX, BX
@@ -93,6 +85,11 @@ rem_loop:
 end:
     RET
 
+one_crc:
+    MOVQ    $1, R13
+    XORQ    BX, BX  
+    JMP     rem_loop
+
 
 // func matchLenSSE4(a, b []byte, max int) int
 TEXT ·matchLenSSE4(SB), 7, $0

flate/deflate_test.go

@@ -99,24 +99,61 @@ func TestCRCBulk(t *testing.T) {
 	}
 	for _, x := range deflateTests {
 		y := x.out
-		if len(y) >= minMatchLength {
+		y = append(y, y...)
+		y = append(y, y...)
+		y = append(y, y...)
+		y = append(y, y...)
+		y = append(y, y...)
+		y = append(y, y...)
+		if !testing.Short() {
 			y = append(y, y...)
-			for j := 4; j < len(y); j++ {
-				y := y[:j]
-				dst := make([]hash, len(y)-minMatchLength+1)
+			y = append(y, y...)
+		}
+		y = append(y, 1)
+		if len(y) >= minMatchLength {
+			for j := len(y) - 1; j >= 4; j-- {
+
+				// Create copy, so we easier detect of-of-bound reads
+				test := make([]byte, j)
+				test2 := make([]byte, j)
+				copy(test, y[:j])
+				copy(test2, y[:j])
+
+				// We allocate one more than we need to test for unintentional overwrites
+				dst := make([]hash, j-3+1)
+				ref := make([]hash, j-3+1)
 				for i := range dst {
 					dst[i] = hash(i + 100)
+					ref[i] = hash(i + 101)
 				}
-				crc32sseAll(y, dst)
+				// Last entry must NOT be overwritten.
+				dst[j-3] = 0x1234
+				ref[j-3] = 0x1234
+
+				// Do two encodes we can compare
+				crc32sseAll(test, dst)
+				crc32sseAll(test2, ref)
+
+				// Check all values
 				for i, got := range dst {
-					expect := crc32sse(y[i:])
+					if i == j-3 {
+						if dst[i] != 0x1234 {
+							t.Fatalf("end of expected dst overwritten, was %08x", uint32(dst[i]))
+						}
+						continue
+					}
+					expect := crc32sse(y[i : i+4])
 					if got != expect && got == hash(i)+100 {
-						t.Errorf("Len:%d Index:%d, expected 0x%08x but not modified", len(y), i, expect)
+						t.Errorf("Len:%d Index:%d, expected 0x%08x but not modified", len(y), i, uint32(expect))
 					} else if got != expect {
-						t.Errorf("Len:%d Index:%d, got 0x%08x expected:0x%08x", len(y), i, got, expect)
+						t.Errorf("Len:%d Index:%d, got 0x%08x expected:0x%08x", len(y), i, uint32(got), uint32(expect))
 					} else {
 						//t.Logf("Len:%d Index:%d OK", len(y), i)
 					}
+					expect = ref[i]
+					if got != expect {
+						t.Errorf("Len:%d Index:%d, got 0x%08x expected:0x%08x", len(y), i, got, expect)
+					}
 				}
 			}
 		}