diff --git a/contracts/utils/cryptography/WebAuthn.sol b/contracts/utils/cryptography/WebAuthn.sol index 7ca254d7..4406e68e 100644 --- a/contracts/utils/cryptography/WebAuthn.sol +++ b/contracts/utils/cryptography/WebAuthn.sol @@ -60,11 +60,6 @@ library WebAuthn { /// @dev Bit 4 of the authenticator data flags: "Backup State" bit. bytes1 private constant AUTH_DATA_FLAGS_BS = 0x10; - /// @dev The expected type string in the client data JSON when verifying assertion signatures. - /// https://www.w3.org/TR/webauthn-2/#dom-collectedclientdata-type - // solhint-disable-next-line quotes - bytes32 private constant EXPECTED_TYPE_HASH = keccak256('"type":"webauthn.get"'); - /** * @dev Performs the absolute minimal verification of a WebAuthn Authentication Assertion. * This function includes only the essential checks required for basic WebAuthn security: @@ -89,18 +84,16 @@ library WebAuthn { // - 32 bytes for rpIdHash // - 1 byte for flags // - 4 bytes for signature counter - if (auth.authenticatorData.length < 37) return false; - bytes memory clientDataJSON = bytes(auth.clientDataJSON); - return - validateExpectedTypeHash(clientDataJSON, auth.typeIndex) && // 11 - validateChallenge(clientDataJSON, auth.challengeIndex, challenge) && // 12 + auth.authenticatorData.length > 36 && + validateExpectedTypeHash(auth.clientDataJSON, auth.typeIndex) && // 11 + validateChallenge(auth.clientDataJSON, auth.challengeIndex, challenge) && // 12 // Handles signature malleability internally P256.verify( sha256( abi.encodePacked( auth.authenticatorData, - sha256(clientDataJSON) // 19 + sha256(bytes(auth.clientDataJSON)) // 19 ) ), auth.r, @@ -222,29 +215,64 @@ library WebAuthn { * @dev Validates that the https://www.w3.org/TR/webauthn-2/#type[Type] field in the client data JSON * is set to "webauthn.get". */ - function validateExpectedTypeHash(bytes memory clientDataJSON, uint256 typeIndex) internal pure returns (bool) { + function validateExpectedTypeHash(string memory clientDataJSON, uint256 typeIndex) internal pure returns (bool) { // 21 = length of '"type":"webauthn.get"' - bytes memory typeValueBytes = Bytes.slice(clientDataJSON, typeIndex, typeIndex + 21); - return keccak256(typeValueBytes) == EXPECTED_TYPE_HASH; + bytes memory typeValueBytes = Bytes.slice(bytes(clientDataJSON), typeIndex, typeIndex + 21); + + // solhint-disable-next-line quotes + return bytes21(typeValueBytes) == bytes21('"type":"webauthn.get"'); } /// @dev Validates that the challenge in the client data JSON matches the `expectedChallenge`. function validateChallenge( - bytes memory clientDataJSON, + string memory clientDataJSON, uint256 challengeIndex, - bytes memory expectedChallenge + bytes memory challenge ) internal pure returns (bool) { - bytes memory expectedChallengeBytes = bytes( - // solhint-disable-next-line quotes - string.concat('"challenge":"', Base64.encodeURL(expectedChallenge), '"') - ); - if (challengeIndex + expectedChallengeBytes.length > clientDataJSON.length) return false; - bytes memory actualChallengeBytes = Bytes.slice( - clientDataJSON, - challengeIndex, - challengeIndex + expectedChallengeBytes.length + // solhint-disable-next-line quotes + string memory expectedChallenge = string.concat('"challenge":"', Base64.encodeURL(challenge), '"'); + string memory actualChallenge = string( + Bytes.slice(bytes(clientDataJSON), challengeIndex, challengeIndex + bytes(expectedChallenge).length) ); - return Strings.equal(string(actualChallengeBytes), string(expectedChallengeBytes)); + return Strings.equal(actualChallenge, expectedChallenge); + } + + /** + * @dev Verifies that calldata bytes (`input`) represents a valid `WebAuthnAuth` object. If encoding is valid, + * returns true and the calldata view at the object. Otherwise, returns false and an invalid calldata object. + * + * NOTE: The returned `auth` object should not be accessed if `success` is false. Trying to access the data may + * cause revert/panic. + */ + function tryDecodeAuth(bytes calldata input) internal pure returns (bool success, WebAuthnAuth calldata auth) { + assembly ("memory-safe") { + auth := input.offset + } + + // Minimum length to hold 6 objects (32 bytes each) + if (input.length < 0xC0) return (false, auth); + + // Get offset of non-value-type elements relative to the input buffer + uint256 authenticatorDataOffset = uint256(bytes32(input[0x80:])); + uint256 clientDataJSONOffset = uint256(bytes32(input[0xa0:])); + + // The elements length (at the offset) should be 32 bytes long. We check that this is within the + // buffer bounds. Since we know input.length is at least 32, we can subtract with no overflow risk. + if (input.length - 0x20 < authenticatorDataOffset || input.length - 0x20 < clientDataJSONOffset) + return (false, auth); + + // Get the lengths. offset + 32 is bounded by input.length so it does not overflow. + uint256 authenticatorDataLength = uint256(bytes32(input[authenticatorDataOffset:])); + uint256 clientDataJSONLength = uint256(bytes32(input[clientDataJSONOffset:])); + + // Check that the input buffer is long enough to store the non-value-type elements + // Since we know input.length is at least xxxOffset + 32, we can subtract with no overflow risk. + if ( + input.length - authenticatorDataOffset - 0x20 < authenticatorDataLength || + input.length - clientDataJSONOffset - 0x20 < clientDataJSONLength + ) return (false, auth); + + return (true, auth); } } diff --git a/contracts/utils/cryptography/signers/SignerWebAuthn.sol b/contracts/utils/cryptography/signers/SignerWebAuthn.sol index 78ed7fe0..42b1b767 100644 --- a/contracts/utils/cryptography/signers/SignerWebAuthn.sol +++ b/contracts/utils/cryptography/signers/SignerWebAuthn.sol @@ -40,21 +40,11 @@ abstract contract SignerWebAuthn is SignerP256 { bytes calldata signature ) internal view virtual override returns (bool) { (bytes32 qx, bytes32 qy) = signer(); + (bool decodeSuccess, WebAuthn.WebAuthnAuth calldata auth) = WebAuthn.tryDecodeAuth(signature); return - WebAuthn.verifyMinimal(abi.encodePacked(hash), _toWebAuthnSignature(signature), qx, qy) || - super._rawSignatureValidation(hash, signature); - } - - /// @dev Non-reverting version of signature decoding. - function _toWebAuthnSignature(bytes calldata signature) private pure returns (WebAuthn.WebAuthnAuth memory auth) { - bool decodable; - assembly ("memory-safe") { - let offset := calldataload(signature.offset) - // Validate the offset is within bounds and makes sense for a WebAuthnAuth struct - // A valid offset should be 32 and point to data within the signature bounds - decodable := and(eq(offset, 32), lt(add(offset, 0x80), signature.length)) - } - return decodable ? abi.decode(signature, (WebAuthn.WebAuthnAuth)) : auth; + decodeSuccess + ? WebAuthn.verifyMinimal(abi.encodePacked(hash), auth, qx, qy) + : super._rawSignatureValidation(hash, signature); } } diff --git a/test/helpers/signers.js b/test/helpers/signers.js index 91531e54..92d3567e 100644 --- a/test/helpers/signers.js +++ b/test/helpers/signers.js @@ -92,16 +92,14 @@ class WebAuthnSigningKey extends P256SigningKey { const { r, s } = super.sign(sha256(concat([authenticatorData, sha256(toUtf8Bytes(clientDataJSON))]))); const serialized = AbiCoder.defaultAbiCoder().encode( - ['tuple(bytes32,bytes32,uint256,uint256,bytes,string)'], + ['bytes32', 'bytes32', 'uint256', 'uint256', 'bytes', 'string'], [ - [ - r, - s, - clientDataJSON.indexOf('"challenge"'), - clientDataJSON.indexOf('"type"'), - authenticatorData, - clientDataJSON, - ], + r, + s, + clientDataJSON.indexOf('"challenge"'), + clientDataJSON.indexOf('"type"'), + authenticatorData, + clientDataJSON, ], );