Centrifuge - ravikiranweb3's results

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L67-L77 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L67-L77 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L79-L81

Vulnerability details

Impact

Assigned DOMAIN_SEPARATOR in the constructor is wrong as by that time, the name for the ERC20 token is not assigned which is part of the keccak256 input.

The usage of DOMAIN_SEPARATOR is not evident from the scoped code base, but it could be far reaching if the bytes32 returned is used as a key which will not match the keccak256 generated after assignment.

Proof of Concept

Refer to the code below.

function _calculateDomainSeparator(uint256 chainId) private view returns (bytes32) {
        return keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes(name)),
                keccak256(bytes(version)),
                chainId,
                address(this)
            )
        );
    }

In the constructor, the name is not assigned, but immutable _DOMAIN_SEPARATOR is assigned.

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

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

Tools Used

Manaul Review

Recommended Mitigation Steps

Pass the name of the token as parameter of constructor.

Assessed type

ERC20

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L67-L77 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/LiquidityPool.sol#L220-L226 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/LiquidityPool.sol#L237-L243 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L216-L237

Vulnerability details

Impact

Depositing or redeeming with permit may stop working, if the token name is updated. The digest generation in the permit function of ERC20 takes into account the name of the ERC20 token at the time of generation.

Updating the name of such token with new name using file function could break the signature digests and will not work.

Proof of Concept

Refer to the digest generation in the below function in LiquidityPool comtract.

function permit(address owner, address spender, uint256 value, uint256 deadline, bytes memory signature) public {
        require(block.timestamp <= deadline, "ERC20/permit-expired");
        require(owner != address(0), "ERC20/invalid-owner");

        uint256 nonce;
        unchecked {
            nonce = nonces[owner]++;
        }

        bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR : _calculateDomainSeparator(block.chainid),
                keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, value, nonce, deadline))
            )
        );

        require(_isValidSignature(owner, digest, signature), "ERC20/invalid-permit");

        allowance[owner][spender] = value;
        emit Approval(owner, spender, value);
    }

note how value returned by _calculateDomainSeparator() function above is part of the digest. Note how name is part of the keccack256 which is part of the digest. As such update the name of the ERC20 token will invalidate the digests.

function _calculateDomainSeparator(uint256 chainId) private view returns (bytes32) {
        return keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes(name)),
                keccak256(bytes(version)),
                chainId,
                address(this)
            )
        );
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Assign the name of the token during token in the construction and dont prove a way to change the name later.

Assessed type

ERC20

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/PoolManager.sol#L214-L225 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L67-L77 https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L216-L237

Vulnerability details

Impact

Name of the ERC20 token is part of the digest generated in the permit function of the contract. Any change in the name of the ERC20 will result in a different digest and hence create conflict for permit functions.

updateTrancheTokenMetadata() function in Poolmanager contract allows for updating the name of ERC20 token in the attached Tranche Token contract.

Proof of Concept

The permit function relies on the digest which is generated using _calculateDomainSeparator as one of the parameters. The calculateDomainSeperator uses name of the token as one of the inputs, hence changing the name will impact the digest and hence the permit functionality.

The PoolManager offering the function to update the name of underlying trancheToken will effect the permit based functions.

 function permit(address owner, address spender, uint256 value, uint256 deadline, bytes memory signature) public {
        require(block.timestamp <= deadline, "ERC20/permit-expired");
        require(owner != address(0), "ERC20/invalid-owner");

        uint256 nonce;
        unchecked {
            nonce = nonces[owner]++;
        }

        bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR : _calculateDomainSeparator(block.chainid),
                keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, value, nonce, deadline))
            )
        );

        require(_isValidSignature(owner, digest, signature), "ERC20/invalid-permit");

        allowance[owner][spender] = value;
        emit Approval(owner, spender, value);
    }

Tools Used

Recommended Mitigation Steps

Assessed type

en/de-code