Description

The ecrecover function is used to verify withdrawal signatures. It, however, allows for malleable signatures, as for any given r (the x-coordinate of the signature's ephemeral public key), there exist two valid s values that recover the same address. This, in turn, essentially allows two different signatures to be successfully verified for the same message.

function verifyWithdraw(address sender, EventTypes.WithdrawData memory data) internal view returns (bool) {

    // ... 

    bytes32 hash = keccak256(abi.encodePacked("\x19\x01", eip712DomainHash, hashStruct));
    address signer = ecrecover(hash, data.v, data.r, data.s);

    return signer == sender && signer != address(0);
}

Impact

Although this particular issue does not pose a direct financial risk to the protocol, it does allow for unintended behavior to occur, as two different signatures can be successfully verified for the same message. Moreover, the future usage of ecrecover in other contexts may lead to more severe implications.

Recommendations

We recommend using OpenZeppelin's ECDSA library instead of ecrecover. More information about using the ECDSA library can be found here↗. Essentially, the ECDSA library performs additional checks, such as limiting the s value to the lower half of the curve order as well as limiting the v value to either 27 or 28. These two checks, in turn, prevent the malleability issue from occurring.

Remediation

This issue has been acknowledged by Orderly Network, and a fix was implemented in commit 62abe45d↗.