cryptoauthlib icon indicating copy to clipboard operation
cryptoauthlib copied to clipboard

PKCS11 pkcs11_signature_sign does not check ulDataLen==32 for ECDSA

Open jacobschloss opened this issue 7 months ago • 1 comments

Describe the bug When signing (ECDSA) a too-long buffer via the PKCS11 interface, eg with Botan PKCS11 interface here signing a 33 byte buffer

Botan::PK_Signer signer(atecc_privkey_handle, m_rng, "Raw",  Botan::Signature_Format::Standard, "pkcs11");

std::array<uint8_t, 33> incorrect_long_msg;
incorrect_long_msg.fill(0);
incorrect_long_msg[32] = 1;
pkcs11sig = signer.sign_message(incorrect_long_msg, m_rng);

"C_Sign" / pkcs11_signature_sign is passed in the length of the buffer from the calling 3rd party library, checks if ulDataLen is 0, but not if ulDataLen is 32 which atcab_sign_ext assumes.

Later we can match the signature by only passing in the first 32 bytes

Botan::PK_Verifier sig_verifier(*tmpkey, "Raw(SHA-256)", Botan::Signature_Format::Standard);

std::array<uint8_t, 32> msg;
msg.fill(0);
sig_verifier.verify_message(msg, pkcs11sig)) < -- will return true, since the last byte in the above signing operation was silently ignored.

Expected behavior Input message buffer length to be validated and an error code to be returned (maybe CKR_DATA_LEN_RANGE or CKR_ARGUMENTS_BAD?) since pkcs11_signature_sign is told the buffer length and knows it is an incorrect size for the requested signing operation.

Additional context If a buffer is passed in that is less than 32 bytes long and near a section boundary, presumably the program could crash as atcab_sign_ext will try to read 32 bytes.

The function pkcs11_signature_check_params checks the length of the signature output buffer, but not the length of the input message buffer. Adding the precondition check to this function could also make sense.

jacobschloss avatar Jul 28 '24 04:07 jacobschloss