sherlock-audit / 2022-10-mover-judging

1 stars 0 forks source link

sorrynotsorry - ecrecover might fail silently #98

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

sorrynotsorry

informational

ecrecover might fail silently

Summary

The ecrecover precompile might fail silently and just returns the zero address as signer when given malformed messages.

Vulnerability Detail

It is important to ensure signer != address(0) to avoid if somehow an arbitrary message was managed to be signed by 0 address.

Impact

Informational

Code Snippet

    function recoverSigner(bytes32 message, bytes memory sig) internal pure returns (address)
    {
        // signature is expected to be exactly 65 bytes (2 * 32 byte words and a checksum)
        require(sig.length == 65, "invalid sig length");

        // signature components
        bytes32 r;
        bytes32 s;
        uint8 v;
        assembly {
            // first 32 bytes, after the length prefix
            r := mload(add(sig, 32))
            // second 32 bytes
            s := mload(add(sig, 64))
            // final byte (first byte of the next 32 bytes)
            v := byte(0, mload(add(sig, 96)))
        }

        // Version of signature should be 27 or 28, but 0 and 1 are also possible versions
        if (v < 27) {
            v += 27;
        }

        return ecrecover(message, v, r, s);
    }

Permalink

Tool used

Manual Review

Recommendation

It's a good practice to include;

require(signer != address(0))