PKCS7 unpadding: signed-byte comparison, zero pad value not rejected, silent failure

[MarkAtwood]

Summary

BlockCipher.unPadPKCS7() has three bugs found by Wycheproof:

1. Signed-byte comparison — padValue is stored as byte, so values 0x81–0xFF compare incorrectly against blockSize. The check (padValue & 0xff) > blockSize works around it for the range check but the loop in[i-1] != padValue does not — a pad byte of e.g. 0x81 will be compared as -127.

2. Zero pad value not rejected — PKCS7 pad values must be in [1, blockSize]. A pad byte of 0x00 passes the > blockSize check and produces a zero-length removal, silently accepting invalid padding.

3. Silent failure — when pad bytes are inconsistent the method sets valid = false but continues and returns the (incorrectly) unpadded data instead of throwing. Callers that don't check the return value get corrupted plaintext.

Additionally, WolfCryptCipher.engineDoFinal() does not reject an empty ciphertext with PKCS5 padding (which cannot contain valid padding).

Impact

Incorrect padding acceptance can enable padding oracle attacks.

Fix

• Store padValue as int using in[in.length - 1] & 0xFF
• Reject padValue < 1 || padValue > blockSize
• Throw WolfCryptException (not set a flag) when pad bytes are inconsistent
• In WolfCryptCipher, throw BadPaddingException when total decrypted size is 0 with PKCS5 padding

Discovered by

Wycheproof test suite (testAesCbcPkcs5 and related vectors).