asDinstances are vulnerable to reorg attack

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

asDcontract instances can be hijacked by attackers in case of a reorg and the interests earned in these contracts will be claimed by the attacker.

Proof of Concept

This protocol allows anyone to create asD contract instances with the asDFactory.sol. These instances are created with the new keyword without adding a salt, which means they are built with the create opcode and the deployed addresses only depend on the nonce of the factory.
https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/asD/src/asDFactory.sol#L33C1-L38C6

file: asDFactory.sol
    function create(string memory _name, string memory _symbol) external returns (address) {
-->     asD createdToken = new asD(_name, _symbol, msg.sender, cNote, owner()); //@audit it doesn't use a salt. "_name" and "_symbol" also doesn't have to be uniqe.
        isAsD[address(createdToken)] = true;
        emit CreatedToken(address(createdToken), _symbol, _name, msg.sender);
        return address(createdToken);
    }

The second thing I would like to point out is that the _name and the _symbol don't have to be unique when creating asD's. Reference in the Code4rena contest page:
"The asDFactory is used to create a new asD tokens. It only exposes a function create that accepts the name and symbol of the token to create. These do not have to be unique."

These two situations combined make the protocol vulnerable to reorgs, which can happen in any EVM chain.

1. Alice is an artist who plans to use ERC1155tech of this protocol for her NFT art project (note: ERC1155tech protocol uses asD as currency).

2. Alice creates an asD contract, and uses this newly deployed contract's address in her art project as payment currency.

3. Bob sees a network reorg happens and creates an asD token with the same name and the same symbol.

4. Bob's transaction is executed first, and now Bob is the owner of the asD contract, which is deployed in the same address that Alice already started to use.

5. Alice's transaction to create asD executes after Bob's but does not revert as the name and the symbol doesn't have to be unique.

6. Alice's asD instance is different than her previous one, but she can't be aware of this because
   - Her transaction didn't revert and she didn't get alarmed
   - Everything is the same except the owner. The same asD address, the same _name, and the same _symbol. There is nothing to be suspicious about.

7. Alice's art collection reaches good volumes and her asD contract earns interest. However, she can not withdraw these as she is not the actual owner of that asD contract.

Tools Used

Manual review

Recommended Mitigation Steps

I would recommend using the new keyword with a salt derived from the msg.sender

For example:

function create(string memory _name, string memory _symbol) external returns (address) {
+       bytes32 salt = keccak256(abi.encode(msg.sender));
        // Added salt during creation
+       asD createdToken = new asD{salt: salt}(_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