Centrifuge - gumgumzum's results

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L80 https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L228

Vulnerability details

Impact

Unless ERC20@DOMAIN_SEPARATOR is used or chainid is updated, building the domain separator by reading values directly from the contract will lead to invalid signatures.

The _DOMAIN_SEPARATOR is set in the constructor when the name is still not set for Tranche tokens.

Furthermore, Tranche tokens name can also be updated afterwards via an UpdateTrancheTokenMetadata message.

Proof of Concept

    function testDomainSeparator() public {
        assertEq(token.name(), "");
        
        token.file("name", "DAI");

        assertEq(token.name(), "DAI");

        assertFalse(token.DOMAIN_SEPARATOR() == keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes("")),
                keccak256(bytes(token.version())),
                block.chainid,
                address(token)
            )
        ));

        assertEq(token.DOMAIN_SEPARATOR(), keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes(token.name())),
                keccak256(bytes(token.version())),
                block.chainid,
                address(token)
            )
        ));
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Recache the _DOMAIN_SEPARATOR after a name change

diff --git a/src/token/ERC20.sol b/src/token/ERC20.sol
index 4fbba5e..0e97193 100644
--- a/src/token/ERC20.sol
+++ b/src/token/ERC20.sol
@@ -28,7 +28,7 @@ contract ERC20 is Context {

     // --- EIP712 niceties ---
     uint256 public immutable deploymentChainId;
-    bytes32 private immutable _DOMAIN_SEPARATOR;
+    bytes32 private _DOMAIN_SEPARATOR;
     bytes32 public constant PERMIT_TYPEHASH =
         keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
@@ -81,7 +81,10 @@ contract ERC20 is Context {
     }

     function file(bytes32 what, string memory data) external auth {
-        if (what == "name") name = data;
+        if (what == "name") { 
+            name = data;
+            _DOMAIN_SEPARATOR = _calculateDomainSeparator(deploymentChainId);
+        }
         else if (what == "symbol") symbol = data;
         else revert("ERC20/file-unrecognized-param");
         emit File(what, data);

Assessed type

Other