code review 91320043: crypto/aes: fix overrun in assembly encrypt/decrypt

crypto/aes: fix overrun in assembly encrypt/decrypt

Fixes issue 7928.

[rsc, 2014-05-09 16:39:32]

Hello golang-codereviews@googlegroups.com (cc: agl, bradfitz),

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2014-05-09 16:43:38]

LGTM
On May 9, 2014 9:39 AM, <rsc@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc: agl, bradfitz),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> crypto/aes: fix overrun in assembly encrypt/decrypt
>
> Fixes issue 7928.
>
> Please review this at https://codereview.appspot.com/91320043/
>
> Affected files (+42, -0 lines):
>   M src/pkg/crypto/aes/aes_test.go
>   M src/pkg/crypto/aes/cipher.go
>   M src/pkg/crypto/aes/cipher_asm.go
>
>
> Index: src/pkg/crypto/aes/aes_test.go
> ===================================================================
> --- a/src/pkg/crypto/aes/aes_test.go
> +++ b/src/pkg/crypto/aes/aes_test.go
> @@ -354,6 +354,34 @@
>         }
>  }
>
> +// Test short input/output.
> +// Assembly used to not notice.
> +// See issue 7928.
> +func TestShortBlocks(t *testing.T) {
> +       bytes := func(n int) []byte { return make([]byte, n) }
> +
> +       c, _ := NewCipher(bytes(16))
> +
> +       mustPanic(t, "crypto/aes: input not full block", func() {
> c.Encrypt(bytes(1), bytes(1)) })
> +       mustPanic(t, "crypto/aes: input not full block", func() {
> c.Decrypt(bytes(1), bytes(1)) })
> +       mustPanic(t, "crypto/aes: input not full block", func() {
> c.Encrypt(bytes(100), bytes(1)) })
> +       mustPanic(t, "crypto/aes: input not full block", func() {
> c.Decrypt(bytes(100), bytes(1)) })
> +       mustPanic(t, "crypto/aes: output not full block", func() {
> c.Encrypt(bytes(1), bytes(100)) })
> +       mustPanic(t, "crypto/aes: output not full block", func() {
> c.Decrypt(bytes(1), bytes(100)) })
> +}
> +
> +func mustPanic(t *testing.T, msg string, f func()) {
> +       defer func() {
> +               err := recover()
> +               if err == nil {
> +                       t.Errorf("function did not panic, wanted %q", msg)
> +               } else if err != msg {
> +                       t.Errorf("got panic %v, wanted %q", err, msg)
> +               }
> +       }()
> +       f()
> +}
> +
>  func BenchmarkEncrypt(b *testing.B) {
>         tt := encryptTests[0]
>         c, err := NewCipher(tt.key)
> Index: src/pkg/crypto/aes/cipher.go
> ===================================================================
> --- a/src/pkg/crypto/aes/cipher.go
> +++ b/src/pkg/crypto/aes/cipher.go
> @@ -46,9 +46,21 @@
>  func (c *aesCipher) BlockSize() int { return BlockSize }
>
>  func (c *aesCipher) Encrypt(dst, src []byte) {
> +       if len(src) < BlockSize {
> +               panic("crypto/aes: input not full block")
> +       }
> +       if len(dst) < BlockSize {
> +               panic("crypto/aes: output not full block")
> +       }
>         encryptBlock(c.enc, dst, src)
>  }
>
>  func (c *aesCipher) Decrypt(dst, src []byte) {
> +       if len(src) < BlockSize {
> +               panic("crypto/aes: input not full block")
> +       }
> +       if len(dst) < BlockSize {
> +               panic("crypto/aes: output not full block")
> +       }
>         decryptBlock(c.dec, dst, src)
>  }
> Index: src/pkg/crypto/aes/cipher_asm.go
> ===================================================================
> --- a/src/pkg/crypto/aes/cipher_asm.go
> +++ b/src/pkg/crypto/aes/cipher_asm.go
> @@ -21,6 +21,7 @@
>                 encryptBlockGo(xk, dst, src)
>         }
>  }
> +
>  func decryptBlock(xk []uint32, dst, src []byte) {
>         if useAsm {
>                 decryptBlockAsm(len(xk)/4-1, &xk[0], &dst[0], &src[0])
> @@ -28,6 +29,7 @@
>                 decryptBlockGo(xk, dst, src)
>         }
>  }
> +
>  func expandKey(key []byte, enc, dec []uint32) {
>         if useAsm {
>                 rounds := 10
>
>
>

[rsc, 2014-05-09 19:40:57]

*** Submitted as https://code.google.com/p/go/source/detail?r=ba0d1163c276 ***

crypto/aes: fix overrun in assembly encrypt/decrypt

Fixes issue 7928.

LGTM=bradfitz
R=golang-codereviews
CC=agl, bradfitz, golang-codereviews
https://codereview.appspot.com/91320043

[agl1, 2014-05-09 19:41:09]

LGTM