Merge pull request #80 from gcash/validate-pkcs7

[Fix] Validate PKCS7 padding correctly
gcash · Oct 27, 2018 · 77333ba · 77333ba
2 parents 021c797 + 68ae405
commit 77333ba
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 1 deletion.
diff --git a/bchec/ciphering.go b/bchec/ciphering.go
@@ -208,7 +208,15 @@ func addPKCSPadding(src []byte) []byte {
 func removePKCSPadding(src []byte) ([]byte, error) {
  length := len(src)
  padLength := int(src[length-1])
- if padLength > aes.BlockSize || length < aes.BlockSize {
+
+ // Padding length must be between 1 and aes block size (16), else invalid:
+ if padLength > aes.BlockSize || length < aes.BlockSize || padLength == 0 {
+ return nil, errInvalidPadding
+ }
+
+ // Padding must contain the padding length byte, repeated (RFC2315 10.3.2)
+ expectedPadding := bytes.Repeat(src[length-1:], padLength)
+ if !bytes.Equal(src[length-padLength:length], expectedPadding) {
  return nil, errInvalidPadding
  }
 

diff --git a/bchec/ciphering_test.go b/bchec/ciphering_test.go
@@ -162,8 +162,35 @@ func TestCipheringErrors(t *testing.T) {
  tests2 := []struct {
  in []byte // input data
  }{
+ // too long
  {bytes.Repeat([]byte{0x11}, 17)},
+ // too short
  {bytes.Repeat([]byte{0x07}, 15)},
+ // invalid padding
+ {append(bytes.Repeat([]byte{0x07}, 15),
+ bytes.Repeat([]byte{0x04}, 1)...)},
+ // invalid padding
+ {append(bytes.Repeat([]byte{0x07}, 9),
+ []byte{0x01, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07}...)},
+ // invalid padding
+ {append(bytes.Repeat([]byte{0x07}, 9),
+ []byte{0x10, 0x10, 0x10, 0x10, 0x10, 0x10, 0xfe, 0x10,
+ 0x10, 0x10, 0x10, 0x10, 0x10, 0x10, 0x10, 0x10}...)},
+ // invalid padding
+ {append(bytes.Repeat([]byte{0x07}, 9),
+ []byte{0x01, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07}...)},
+ // invalid padding
+ {append(bytes.Repeat([]byte{0x07}, 9),
+ []byte{0x01, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07}...)},
+ // invalid padding length
+ {append(bytes.Repeat([]byte{0x07}, 15),
+ bytes.Repeat([]byte{0x11}, 1)...)},
+ // invalid padding length
+ {append(bytes.Repeat([]byte{0x07}, 15),
+ bytes.Repeat([]byte{0x00}, 1)...)},
+ // invalid padding length
+ {append(bytes.Repeat([]byte{0x07}, 15),
+ bytes.Repeat([]byte{0xff}, 1)...)},
  }
  for i, test := range tests2 {
  _, err = removePKCSPadding(test.in)