code-423n4 / 2023-09-centrifuge-findings

16 stars 14 forks source link

Risk of reuse of signatures incase of forks #578

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L42-L49

Vulnerability details

Impact

In the event of a hard fork, an attacker could reuse signatures to receive user funds on both chains.

Proof of Concept

At construction, the ERC20.sol contract computes the domain separator using the network’s chainID, which can be changed at the time of re-deployment. In the event of a post-deployment chain fork, the chainID of the network might not change enabling the signatures to be replayed across both versions of the chain.

File: ERC20.sol

       constructor(uint8 decimals_) {
        decimals = decimals_;
        wards[_msgSender()] = 1;
        emit Rely(_msgSender());

        deploymentChainId = block.chainid;
        _DOMAIN_SEPARATOR = _calculateDomainSeparator(block.chainid);
       }

However, the _isValidSignature function checks only the length of the signature which can be easily obtained. As a result, if a fork of a network is made after the contract’s creation, every signature will be usable in both forks.

File: ERC20.sol

     // --- Approve by signature ---
    function _isValidSignature(address signer, bytes32 digest, bytes memory signature) internal view returns (bool) {
        if (signature.length == 65) {
            bytes32 r;
            bytes32 s;
            uint8 v;
            assembly {
                r := mload(add(signature, 0x20))
                s := mload(add(signature, 0x40))
                v := byte(0, mload(add(signature, 0x60)))
            }
            if (signer == ecrecover(digest, v, r, s)) {
                return true;
            }
        }

Take the following scenario,

Tools Used

Manual Review

Recommended Mitigation Steps

Identify and document the risks associated with having forks of multiple chains and develop related mitigation strategies.

Assessed type

Upgradable

raymondfam commented 1 year ago

From the doc:

The Centrifuge Protocol is built on Centrifuge Chain, a layer 1 blockchain custom built for Real World Assets, using the Substrate framework. Key advantages of Substrate include shared security as a parachain, built-in on-chain governance and forkless upgrades, and trustless bridging with other parachains. These enable Centrifuge to focus on building Centrifuge Chain into the go-to place for bringing real world assets on-chain.

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as primary issue

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Out of scope