xz: fix a security issue for readUvarint

readUvarint could be provided a sequence of bytes where the application
would never stop. That is the same issue that has been recently
reported for the Go Standard Library as CVE-2020-16845.

The fix simply adds a check for the number of bytes read and reports an
overflow after more than 10 bytes are read, which is
$\ceil{\frac{64}{7}}$.

The commit also includes a test to ensure that the error is returned.

I thank Github user 0xdecaf for reporting the issue.

Author: ulikunitz

bits.go

@@ -54,6 +54,8 @@ var errOverflowU64 = errors.New("xz: uvarint overflows 64-bit unsigned integer")
 
 // readUvarint reads a uvarint from the given byte reader.
 func readUvarint(r io.ByteReader) (x uint64, n int, err error) {
+	const maxUvarintLen = 10
+
 	var s uint
 	i := 0
 	for {
@@ -62,8 +64,11 @@ func readUvarint(r io.ByteReader) (x uint64, n int, err error) {
 			return x, i, err
 		}
 		i++
+		if i > maxUvarintLen {
+			return x, i, errOverflowU64
+		}
 		if b < 0x80 {
-			if i > 10 || i == 10 && b > 1 {
+			if i == maxUvarintLen && b > 1 {
 				return x, i, errOverflowU64
 			}
 			return x | uint64(b)<<s, i, nil

bits_test.go

@@ -31,3 +31,14 @@ func TestUvarint(t *testing.T) {
 		}
 	}
 }
+
+func TestUvarIntCVE_2020_16845(t *testing.T) {
+	var a = []byte{0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,
+		0x88, 0x89, 0x8a, 0x8b}
+
+	r := bytes.NewReader(a)
+	_, _, err := readUvarint(r)
+	if err != errOverflowU64 {
+		t.Fatalf("readUvarint overflow not detected")
+	}
+}