`create` methods are suspicious of the reorg attack in `asDFactory.sol`

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/asD/src/asDFactory.sol#L34

Vulnerability details

Impact

asDFactory.sol is factory contract used for creating application-specific dollars(asD). asDFactory.create() function is used to deploy a new asD and msg.sender is set as owner by default while creating the asD using the create opcode, where the address derivation depends only on the asDFactory.sol nonce. In ethereum, Re-orgs has already been happened. For more info, check below links

https://cointelegraph.com/news/ethereum-beacon-chain-experiences-7-block-reorg-what-s-going-on https://decrypt.co/101390/ethereum-beacon-chain-blockchain-reorg

It should be noted that, the contracts will be deployed on CANTO which is a permissionless general-purpose blockchain running the Ethereum Virtual Machine (EVM). Reorg has been happened on Ethereum, polygon in past and the issue can also be considered for other chains.

The issue would happen when users rely on the address derivation in advance or try to deploy the position clone with the same address on different EVM chains, any funds sent to the new contract could potentially be withdrawn by anyone else. All in all, it could lead to the theft of user funds.

File: asD/src/asDFactory.sol

    function create(string memory _name, string memory _symbol) external returns (address) {
>>      asD createdToken = new asD(_name, _symbol, msg.sender, cNote, owner());   @audit // using create opcode
        isAsD[address(createdToken)] = true;
        emit CreatedToken(address(createdToken), _symbol, _name, msg.sender);
        return address(createdToken);
    }

create() can be called by anyone and it does not have access control.

    mapping(address => bool) public isAsD;

When the asD is created by create opcode and it is stored in mapping isAsD which allows integrating contracts to query if a given address is a legit asD.

Attack Scenario Imagine that Alice deploys a new asD, and then sends funds to it. Bob sees that the network block reorg happens and calls create(). Thus, it creates asD with an address to which Alice sends funds. Then Alice's transactions are executed and Alice transfers funds to Bob’s controlled asD.

Proof of Concept

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/asD/src/asDFactory.sol#L34

Tools Used

Manual review

Recommended Mitigation Steps

Deploy such contracts via create2 with salt that includes msg.sender. Additionally deployer nonce can also be included.

File: asD/src/asDFactory.sol

+    /// @notice Mapping to store deployer nonces for CREATE2
+    mapping(address deployer => uint256 nonce) public deployerNonces;

    function create(string memory _name, string memory _symbol) external returns (address) {
-      asD createdToken = new asD(_name, _symbol, msg.sender, cNote, owner()); 
+      asD createdToken = new asD{salt: keccak256(abi.encode(msg.sender, deployerNonces[msg.sender]++))}(_name, _symbol, msg.sender, cNote, owner()); 
        isAsD[address(createdToken)] = true;
        emit CreatedToken(address(createdToken), _symbol, _name, msg.sender);
        return address(createdToken);
    }

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as duplicate of #313

[c4-judge]

MarioPoneder changed the severity to QA (Quality Assurance)

[c4-judge]

MarioPoneder marked the issue as grade-c

[c4-judge]

MarioPoneder marked the issue as grade-b