Potential Vulnerability Due to Behavior of slice Function in the WebAuthn.sol

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-03-coinbase/blob/e0573369b865d47fed778de00a7b6df65ab1744e/src/WebAuthnSol/WebAuthn.sol#L104-L162

Vulnerability details

Impact

The WebAuthn.sol contract which is a very critical component of the smart wallet system and is designed to verify WebAuthn Authentication Assertions. The contract's current implementation does not explicitly validate the length of the clientDataJSON string or the extracted slices, such as _type. This could potentially enable an attacker to exploit the contract by providing an empty or malformed clientDataJSON string that would still pass the validation checks.

To add more context, slice function in Solidity does not inherently validate the length of the extracted substring; it simply extracts a substring based on the provided indices. If the clientDataJSON string is shorter than expected, the slice function will return an empty string or a string shorter than expected, which could still pass the keccak256 check if the expected hash value is also derived from an empty or shorter string 45.

The keccak256 hash function is deterministic and will produce the same output for different inputs of the same length, even if the content is different. This means that if an attacker can manipulate the input string to be of the same length as the expected input, they could potentially bypass the validation checks by providing a malformed or empty clientDataJSON string that still results in a valid keccak256 hash when compared against EXPECTED_TYPE_HASH

Contract Analysis:

The WebAuthn.sol contract includes a verify function that processes a WebAuthnAuth struct, which contains a clientDataJSON string. This string is sliced to extract the type and challenge, but the contract lacks explicit checks to ensure that the clientDataJSON string is of the expected length or that the extracted slices are not empty.

https://github.com/code-423n4/2024-03-coinbase/blob/e0573369b865d47fed778de00a7b6df65ab1744e/src/WebAuthnSol/WebAuthn.sol#L116-L119

string memory _type = webAuthnAuth.clientDataJSON.slice(webAuthnAuth.typeIndex, webAuthnAuth.typeIndex + 21);
if (keccak256(bytes(_type)) != EXPECTED_TYPE_HASH) {
    return false;
}

Potential Exploit:

An attacker could provide an empty or malformed clientDataJSON string that would still pass the keccak256 check against EXPECTED_TYPE_HASH. This could allow the attacker to bypass the verification process and potentially exploit the contract.

POC Test Contract:

To demonstrate the potential issue, a test contract could be created that attempts to call the verify function with an empty or malformed clientDataJSON string. The test contract should be designed to pass the keccak256 check but fail the length check, thus demonstrating the vulnerability.

contract WebAuthnTest {
    WebAuthn webAuthn;

    constructor(WebAuthn _webAuthn) {
        webAuthn = _webAuthn;
    }

    function testVerifyWithEmptyClientDataJSON() public {
        WebAuthn.WebAuthnAuth memory webAuthnAuth;
        webAuthnAuth.clientDataJSON = ""; // Empty clientDataJSON string
        // Set other fields as needed

        bool result = webAuthn.verify(/* parameters */);
        assert(!result); // Expect the verification to fail
    }

    function testVerifyWithMalformedClientDataJSON() public {
        WebAuthn.WebAuthnAuth memory webAuthnAuth;
        webAuthnAuth.clientDataJSON = "malformed"; // Malformed clientDataJSON string
        // Set other fields as needed

        bool result = webAuthn.verify(/* parameters */);
        assert(!result); // Expect the verification to fail
    }
}

Recommended Mitigation Steps:

Explicit length checks for the clientDataJSON string and the extracted slices should be included. This can be done by adding require statements to ensure that the clientDataJSON string is not empty and that the extracted slices are of the expected length.

require(webAuthnAuth.clientDataJSON.length >= webAuthnAuth.typeIndex + 21, "Invalid clientDataJSON length");
string memory _type = webAuthnAuth.clientDataJSON.slice(webAuthnAuth.typeIndex, webAuthnAuth.typeIndex + 21);
require(_type.length == 21, "Invalid type length");
if (keccak256(bytes(_type)) != EXPECTED_TYPE_HASH) {
    return false;
}

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Verification check should be QA at best considering the call would still fail and no exploit could be made.

[3docSec]

The core claim:

An attacker could provide an empty or malformed clientDataJSON string that would still pass the keccak256 check against EXPECTED_TYPE_HASH. This could allow the attacker to bypass the verification process and potentially exploit the contract.

is not true. All the if in the verify functions end with a return false;, so there is no opportunity to "bypass the signature verification".

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid