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>

diff --git a/src/crypto/aes/ctr_s390x.go b/src/crypto/aes/ctr_s390x.go
index 94dea5ccdfee08159a96b790856c91abc38144e5..8078aa68026c3777b2dd7bed011f62477411ea90 100644 (file)
--- a/src/crypto/aes/ctr_s390x.go
+++ b/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()

diff --git a/src/crypto/cipher/xor.go b/src/crypto/cipher/xor.go
index 01ca0a9f086f8bf8bda27460b088c2c23a7d43bf..5b26eace09d8bd98919c8335ab1f060bb937fa48 100644 (file)
--- a/src/crypto/cipher/xor.go
+++ b/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)

diff --git a/src/crypto/issue21104_test.go b/src/crypto/issue21104_test.go
new file mode 100644 (file)
index 0000000..b4276df
--- /dev/null
+++ 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)
+}

diff --git a/src/crypto/rc4/rc4_asm.go b/src/crypto/rc4/rc4_asm.go
index 02e5b67d55385f604a26711d206ed1e105a818ae..8d464547fa86d023859d996f959430e84578ee70 100644 (file)
--- a/src/crypto/rc4/rc4_asm.go
+++ b/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)
 }