Skip to content

Upgrade signature validation#

Medium Risk


Fixed according to the recommendation.

This is the code in OrderValidator.sol that validates signatures:

bytes32 hash = LibOrder.hash(order);
if (_hashTypedDataV4(hash).recover(signature) != order.maker) {
    if (order.maker.isContract()) {
            ERC1271(order.maker).isValidSignature(_hashTypedDataV4(hash), signature) == MAGICVALUE,
            "contract order signature verification error"
    } else {
        revert("order signature verification error");

recover reverts internally so isValidSignature can't be called#

A problem with this code is that the recover() function reverts internally if it determines a signature to be invalid.

It is possible for ERC1271(order.maker).isValidSignature to validate a signature differently. What may be valid for ERC1271(order.maker).isValidSignature may not be valid for recover.

But ERC1271(order.maker).isValidSignature will never get a chance to validate signatures if recover reverts.

recover does not support short signatures#

The recover function comes from LibSignature.sol which is coped from ECDSAUpgradeable.sol#release-v4.3, which is an older version that does not support short signatures.

Current versions of recover in ECDSAUpgradeable.sol from OpenZeppelin support short signatures.


Delete LibSignature.sol and use the isValidSignatureNow function from SignatureCheckerUpgradeable.sol instead.

The isValidSignatureNow function combines the recover and isValidSignature function calls into a single function call in a way that solves Item 1 and Item 2 mentioned above.

The code could look like this:

At the top of the OrderValidate.sol file import SignatureCheckerUpgradeable:

import "@openzeppelin/contracts-upgradeable/utils/cryptography/SignatureCheckerUpgradeable.sol";
Then use it:

bytes32 hash = LibOrder.hash(order);
require(SignatureCheckerUpgradeable.isValidSignatureNow(order.maker, _hashTypedDataV4(hash), signature), "order signature verification error");