crypto: panic on illegal input and output overlap

Normalized all panic checks and added inexact aliasing panics across Stream, Block, BlockMode and AEAD implementations. Also, tweaked the aliasing docs of cipher.AEAD, as they did not account for the append nature of the API. Fixes golang#21624 Change-Id: I075c4415f59b3c06e3099bd9f76de6d12af086bf Reviewed-on: https://go-review.googlesource.com/109697 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
yuguangh · Jun 19, 2018 · 75d15a2 · 75d15a2
1 parent c6e455b
commit 75d15a2
Show file tree

Hide file tree

Showing 21 changed files with 366 additions and 87 deletions.
diff --git a/src/crypto/aes/aes_gcm.go b/src/crypto/aes/aes_gcm.go
@@ -8,6 +8,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	subtleoverlap "crypto/internal/subtle"
 	"crypto/subtle"
 	"errors"
 )
@@ -99,10 +100,10 @@ func sliceForAppend(in []byte, n int) (head, tail []byte) {
 // details.
 func (g *gcmAsm) Seal(dst, nonce, plaintext, data []byte) []byte {
 	if len(nonce) != g.nonceSize {
-		panic("cipher: incorrect nonce length given to GCM")
+		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 	if uint64(len(plaintext)) > ((1<<32)-2)*BlockSize {
-		panic("cipher: message too large for GCM")
+		panic("crypto/cipher: message too large for GCM")
 	}
 
 	var counter, tagMask [gcmBlockSize]byte
@@ -123,6 +124,9 @@ func (g *gcmAsm) Seal(dst, nonce, plaintext, data []byte) []byte {
 	gcmAesData(&g.productTable, data, &tagOut)
 
 	ret, out := sliceForAppend(dst, len(plaintext)+g.tagSize)
+	if subtleoverlap.InexactOverlap(out[:len(plaintext)], plaintext) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 	if len(plaintext) > 0 {
 		gcmAesEnc(&g.productTable, out, plaintext, &counter, &tagOut, g.ks)
 	}
@@ -136,12 +140,12 @@ func (g *gcmAsm) Seal(dst, nonce, plaintext, data []byte) []byte {
 // for details.
 func (g *gcmAsm) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
 	if len(nonce) != g.nonceSize {
-		panic("cipher: incorrect nonce length given to GCM")
+		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 	// Sanity check to prevent the authentication from always succeeding if an implementation
 	// leaves tagSize uninitialized, for example.
 	if g.tagSize < gcmMinimumTagSize {
-		panic("cipher: incorrect GCM tag size")
+		panic("crypto/cipher: incorrect GCM tag size")
 	}
 
 	if len(ciphertext) < g.tagSize {
@@ -173,6 +177,9 @@ func (g *gcmAsm) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
 	gcmAesData(&g.productTable, data, &expectedTag)
 
 	ret, out := sliceForAppend(dst, len(ciphertext))
+	if subtleoverlap.InexactOverlap(out, ciphertext) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 	if len(ciphertext) > 0 {
 		gcmAesDec(&g.productTable, out, ciphertext, &counter, &expectedTag, g.ks)
 	}

diff --git a/src/crypto/aes/cbc_s390x.go b/src/crypto/aes/cbc_s390x.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 )
 
 // Assert that aesCipherAsm implements the cbcEncAble and cbcDecAble interfaces.
@@ -48,6 +49,9 @@ func (x *cbc) CryptBlocks(dst, src []byte) {
 	if len(dst) < len(src) {
 		panic("crypto/cipher: output smaller than input")
 	}
+	if subtle.InexactOverlap(dst[:len(src)], src) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 	if len(src) > 0 {
 		cryptBlocksChain(x.c, &x.iv[0], &x.b.key[0], &dst[0], &src[0], len(src))
 	}

diff --git a/src/crypto/aes/cipher.go b/src/crypto/aes/cipher.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 	"strconv"
 )
 
@@ -57,6 +58,9 @@ func (c *aesCipher) Encrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	encryptBlockGo(c.enc, dst, src)
 }
 
@@ -67,5 +71,8 @@ func (c *aesCipher) Decrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	decryptBlockGo(c.dec, dst, src)
 }
diff --git a/src/crypto/aes/cipher_amd64.go b/src/crypto/aes/cipher_amd64.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 	"internal/cpu"
 )
 
@@ -52,6 +53,9 @@ func (c *aesCipherAsm) Encrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	encryptBlockAsm(len(c.enc)/4-1, &c.enc[0], &dst[0], &src[0])
 }
 
@@ -62,6 +66,9 @@ func (c *aesCipherAsm) Decrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	decryptBlockAsm(len(c.dec)/4-1, &c.dec[0], &dst[0], &src[0])
 }
 

diff --git a/src/crypto/aes/cipher_arm64.go b/src/crypto/aes/cipher_arm64.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 	"internal/cpu"
 	"math/bits"
 )
@@ -40,6 +41,9 @@ func (c *aesCipherAsm) Encrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	encryptBlockAsm(len(c.enc)/4-1, &c.enc[0], &dst[0], &src[0])
 }
 
@@ -50,6 +54,9 @@ func (c *aesCipherAsm) Decrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	decryptBlockAsm(len(c.dec)/4-1, &c.dec[0], &dst[0], &src[0])
 }
 

diff --git a/src/crypto/aes/cipher_ppc64le.go b/src/crypto/aes/cipher_ppc64le.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 )
 
 // defined in asm_ppc64le.s
@@ -54,6 +55,9 @@ func (c *aesCipherAsm) Encrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	encryptBlockAsm(&dst[0], &src[0], &c.enc[0])
 }
 
@@ -64,6 +68,9 @@ func (c *aesCipherAsm) Decrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	decryptBlockAsm(&dst[0], &src[0], &c.dec[0])
 }
 

diff --git a/src/crypto/aes/cipher_s390x.go b/src/crypto/aes/cipher_s390x.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 	"internal/cpu"
 )
 
@@ -67,6 +68,9 @@ func (c *aesCipherAsm) Encrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	cryptBlocks(c.function, &c.key[0], &dst[0], &src[0], BlockSize)
 }
 
@@ -77,6 +81,9 @@ func (c *aesCipherAsm) Decrypt(dst, src []byte) {
 	if len(dst) < BlockSize {
 		panic("crypto/aes: output not full block")
 	}
+	if subtle.InexactOverlap(dst[:BlockSize], src[:BlockSize]) {
+		panic("crypto/aes: invalid buffer overlap")
+	}
 	// The decrypt function code is equal to the function code + 128.
 	cryptBlocks(c.function+128, &c.key[0], &dst[0], &src[0], BlockSize)
 }

diff --git a/src/crypto/aes/ctr_s390x.go b/src/crypto/aes/ctr_s390x.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	"crypto/internal/subtle"
 	"unsafe"
 )
 
@@ -64,9 +65,11 @@ func (c *aesctr) refill() {
 }
 
 func (c *aesctr) XORKeyStream(dst, src []byte) {
-	if len(src) > 0 {
-		// Assert len(dst) >= len(src)
-		_ = dst[len(src)-1]
+	if len(dst) < len(src) {
+		panic("crypto/cipher: output smaller than input")
+	}
+	if subtle.InexactOverlap(dst[:len(src)], src) {
+		panic("crypto/cipher: invalid buffer overlap")
 	}
 	for len(src) > 0 {
 		if len(c.buffer) == 0 {

diff --git a/src/crypto/aes/gcm_s390x.go b/src/crypto/aes/gcm_s390x.go
@@ -6,6 +6,7 @@ package aes
 
 import (
 	"crypto/cipher"
+	subtleoverlap "crypto/internal/subtle"
 	"crypto/subtle"
 	"errors"
 	"internal/cpu"
@@ -220,13 +221,16 @@ func (g *gcmAsm) auth(out, ciphertext, additionalData []byte, tagMask *[gcmTagSi
 // details.
 func (g *gcmAsm) Seal(dst, nonce, plaintext, data []byte) []byte {
 	if len(nonce) != g.nonceSize {
-		panic("cipher: incorrect nonce length given to GCM")
+		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 	if uint64(len(plaintext)) > ((1<<32)-2)*BlockSize {
-		panic("cipher: message too large for GCM")
+		panic("crypto/cipher: message too large for GCM")
 	}
 
 	ret, out := sliceForAppend(dst, len(plaintext)+g.tagSize)
+	if subtleoverlap.InexactOverlap(out[:len(plaintext)], plaintext) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 
 	counter := g.deriveCounter(nonce)
 
@@ -246,12 +250,12 @@ func (g *gcmAsm) Seal(dst, nonce, plaintext, data []byte) []byte {
 // for details.
 func (g *gcmAsm) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
 	if len(nonce) != g.nonceSize {
-		panic("cipher: incorrect nonce length given to GCM")
+		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 	// Sanity check to prevent the authentication from always succeeding if an implementation
 	// leaves tagSize uninitialized, for example.
 	if g.tagSize < gcmMinimumTagSize {
-		panic("cipher: incorrect GCM tag size")
+		panic("crypto/cipher: incorrect GCM tag size")
 	}
 	if len(ciphertext) < g.tagSize {
 		return nil, errOpen
@@ -273,6 +277,9 @@ func (g *gcmAsm) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
 	g.auth(expectedTag[:], ciphertext, data, &tagMask)
 
 	ret, out := sliceForAppend(dst, len(ciphertext))
+	if subtleoverlap.InexactOverlap(out, ciphertext) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 
 	if subtle.ConstantTimeCompare(expectedTag[:g.tagSize], tag) != 1 {
 		// The AESNI code decrypts and authenticates concurrently, and
@@ -314,13 +321,16 @@ func kmaGCM(fn code, key, dst, src, aad []byte, tag *[16]byte, cnt *gcmCount)
 // details.
 func (g *gcmKMA) Seal(dst, nonce, plaintext, data []byte) []byte {
 	if len(nonce) != g.nonceSize {
-		panic("cipher: incorrect nonce length given to GCM")
+		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 	if uint64(len(plaintext)) > ((1<<32)-2)*BlockSize {
-		panic("cipher: message too large for GCM")
+		panic("crypto/cipher: message too large for GCM")
 	}
 
 	ret, out := sliceForAppend(dst, len(plaintext)+g.tagSize)
+	if subtleoverlap.InexactOverlap(out[:len(plaintext)], plaintext) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 
 	counter := g.deriveCounter(nonce)
 	fc := g.block.function | kmaLAAD | kmaLPC
@@ -336,7 +346,7 @@ func (g *gcmKMA) Seal(dst, nonce, plaintext, data []byte) []byte {
 // for details.
 func (g *gcmKMA) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
 	if len(nonce) != g.nonceSize {
-		panic("cipher: incorrect nonce length given to GCM")
+		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 	if len(ciphertext) < g.tagSize {
 		return nil, errOpen
@@ -348,9 +358,12 @@ func (g *gcmKMA) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) {
 	tag := ciphertext[len(ciphertext)-g.tagSize:]
 	ciphertext = ciphertext[:len(ciphertext)-g.tagSize]
 	ret, out := sliceForAppend(dst, len(ciphertext))
+	if subtleoverlap.InexactOverlap(out, ciphertext) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 
 	if g.tagSize < gcmMinimumTagSize {
-		panic("cipher: incorrect GCM tag size")
+		panic("crypto/cipher: incorrect GCM tag size")
 	}
 
 	counter := g.deriveCounter(nonce)

diff --git a/src/crypto/cipher/cbc.go b/src/crypto/cipher/cbc.go
@@ -11,6 +11,8 @@
 
 package cipher
 
+import "crypto/internal/subtle"
+
 type cbc struct {
 	b         Block
 	blockSize int
@@ -59,6 +61,9 @@ func (x *cbcEncrypter) CryptBlocks(dst, src []byte) {
 	if len(dst) < len(src) {
 		panic("crypto/cipher: output smaller than input")
 	}
+	if subtle.InexactOverlap(dst[:len(src)], src) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 
 	iv := x.iv
 
@@ -116,6 +121,9 @@ func (x *cbcDecrypter) CryptBlocks(dst, src []byte) {
 	if len(dst) < len(src) {
 		panic("crypto/cipher: output smaller than input")
 	}
+	if subtle.InexactOverlap(dst[:len(src)], src) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 	if len(src) == 0 {
 		return
 	}

diff --git a/src/crypto/cipher/cfb.go b/src/crypto/cipher/cfb.go
@@ -6,6 +6,8 @@
 
 package cipher
 
+import "crypto/internal/subtle"
+
 type cfb struct {
 	b       Block
 	next    []byte
@@ -16,6 +18,12 @@ type cfb struct {
 }
 
 func (x *cfb) XORKeyStream(dst, src []byte) {
+	if len(dst) < len(src) {
+		panic("crypto/cipher: output smaller than input")
+	}
+	if subtle.InexactOverlap(dst[:len(src)], src) {
+		panic("crypto/cipher: invalid buffer overlap")
+	}
 	for len(src) > 0 {
 		if x.outUsed == len(x.out) {
 			x.b.Encrypt(x.out, x.next)