From 68ae405eff0cb38f28767210c73becd4d3cd8f78 Mon Sep 17 00:00:00 2001
From: Josh Ellithorpe <quest@mac.com>
Date: Thu, 25 Oct 2018 21:34:34 -0700
Subject: [PATCH] Validate PKCS7 padding correctly

---
 bchec/ciphering.go      | 10 +++++++++-
 bchec/ciphering_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/bchec/ciphering.go b/bchec/ciphering.go
index fcedeb1fb..c5927586d 100644
--- 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
index 7affe288f..677b0c4c4 100644
--- 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)