crypto/{aes,cipher,rc4}: fix out of bounds write in stream ciphers

Functions XORKeyStream should panic if len(dst) < len(src), but it
write to dst before bounds checking. In asm routines and fastXORBytes,
this is an out of bounds write.

Fixes #21104

Change-Id: I354346cda8d63910f3bb619416ffd54cd0a04a0b
Reviewed-on: https://go-review.googlesource.com/52050
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/crypto/aes/ctr_s390x.go

@@ -64,6 +64,10 @@ func (c *aesctr) refill() {
 }
 
 func (c *aesctr) XORKeyStream(dst, src []byte) {
+	if len(src) > 0 {
+		// Assert len(dst) >= len(src)
+		_ = dst[len(src)-1]
+	}
 	for len(src) > 0 {
 		if len(c.buffer) == 0 {
 			c.refill()

src/crypto/cipher/xor.go

@@ -19,6 +19,11 @@ func fastXORBytes(dst, a, b []byte) int {
 	if len(b) < n {
 		n = len(b)
 	}
+	if n == 0 {
+		return 0
+	}
+	// Assert dst has enough space
+	_ = dst[n-1]
 
 	w := n / wordSize
 	if w > 0 {
@@ -48,8 +53,8 @@ func safeXORBytes(dst, a, b []byte) int {
 	return n
 }
 
-// xorBytes xors the bytes in a and b. The destination is assumed to have enough
-// space. Returns the number of bytes xor'd.
+// xorBytes xors the bytes in a and b. The destination should have enough
+// space, otherwise xorBytes will panic. Returns the number of bytes xor'd.
 func xorBytes(dst, a, b []byte) int {
 	if supportsUnaligned {
 		return fastXORBytes(dst, a, b)

src/crypto/issue21104_test.go

@@ -0,0 +1,61 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package crypto
+
+import (
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rc4"
+	"testing"
+)
+
+func TestRC4OutOfBoundsWrite(t *testing.T) {
+	// This cipherText is encrypted "0123456789"
+	cipherText := []byte{238, 41, 187, 114, 151, 2, 107, 13, 178, 63}
+	cipher, err := rc4.NewCipher([]byte{0})
+	if err != nil {
+		panic(err)
+	}
+	test(t, "RC4", cipherText, cipher.XORKeyStream)
+}
+func TestCTROutOfBoundsWrite(t *testing.T) {
+	testBlock(t, "CTR", cipher.NewCTR)
+}
+func TestOFBOutOfBoundsWrite(t *testing.T) {
+	testBlock(t, "OFB", cipher.NewOFB)
+}
+func TestCFBEncryptOutOfBoundsWrite(t *testing.T) {
+	testBlock(t, "CFB Encrypt", cipher.NewCFBEncrypter)
+}
+func TestCFBDecryptOutOfBoundsWrite(t *testing.T) {
+	testBlock(t, "CFB Decrypt", cipher.NewCFBDecrypter)
+}
+func testBlock(t *testing.T, name string, newCipher func(cipher.Block, []byte) cipher.Stream) {
+	// This cipherText is encrypted "0123456789"
+	cipherText := []byte{86, 216, 121, 231, 219, 191, 26, 12, 176, 117}
+	var iv, key [16]byte
+	block, err := aes.NewCipher(key[:])
+	if err != nil {
+		panic(err)
+	}
+	stream := newCipher(block, iv[:])
+	test(t, name, cipherText, stream.XORKeyStream)
+}
+func test(t *testing.T, name string, cipherText []byte, xor func([]byte, []byte)) {
+	want := "abcdefghij"
+	plainText := []byte(want)
+	shorterLen := len(cipherText) / 2
+	defer func() {
+		err := recover()
+		if err == nil {
+			t.Errorf("%v XORKeyStream expected to panic on len(dst) < len(src), but didn't", name)
+		}
+		const plain = "0123456789"
+		if plainText[shorterLen] == plain[shorterLen] {
+			t.Errorf("%v XORKeyStream did out of bounds write, want %v, got %v", name, want, string(plainText))
+		}
+	}()
+	xor(plainText[:shorterLen], cipherText)
+}

src/crypto/rc4/rc4_asm.go

@@ -14,5 +14,7 @@ func (c *Cipher) XORKeyStream(dst, src []byte) {
 	if len(src) == 0 {
 		return
 	}
+	// Assert len(dst) >= len(src)
+	_ = dst[len(src)-1]
 	xorKeyStream(&dst[0], &src[0], len(src), &c.s, &c.i, &c.j)
 }