Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create can be vulnerable when reorg occurs #315

Open
c4-submissions opened this issue Nov 17, 2023 · 4 comments
Open

Create can be vulnerable when reorg occurs #315

c4-submissions opened this issue Nov 17, 2023 · 4 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-313 grade-b Q-18 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@c4-submissions
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

It's known that the create OP code calculates the address using address = hash(creatorAddr,creatorNonce). This is vulnerable when the network is not that stable and may suffer from Reorg Attack. For chains like Polygon, Op, Arb, reorg could happen and thus unintended situations may occur.

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

Considering that the contract asD created will be used to interact with users for minting, burning, etc, and the create function is external and could be called by anyone, this could lead to the ownership getting lost and later accrued interests being lost.

Proof of Concept

Considering the following situation:
1. Bob has created an asD contract to interact with, so far the nonce is X. At the same time, Alice has also tried to create an asD and the nonce should be X+1.
2. Bob verifies his address on the chain and has asked others (or even himself) to perform mint/burn with this contract so that some accrued interests could be earned.
3. The reorg happens, and Alice's transaction happens before Bob. In this situation, Alice gets nonce=X and has ownership of this address. So Alice could earn what is expected to be given to Bob (the interest that accrued).

Tools Used

Manual

Recommended Mitigation Steps

We recommend using create2 with salt(from msg.sender) to make the address unique for different creators.

Assessed type

Other

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 17, 2023
c4-submissions added a commit that referenced this issue Nov 17, 2023
@c4-pre-sort
Copy link

minhquanym marked the issue as duplicate of #313

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Nov 29, 2023
@c4-judge
Copy link

MarioPoneder changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added grade-c unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Nov 29, 2023
@c4-judge
Copy link

MarioPoneder marked the issue as grade-c

@c4-judge
Copy link

c4-judge commented Dec 4, 2023

MarioPoneder marked the issue as grade-b

@c4-judge c4-judge added grade-b and removed grade-c unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Dec 4, 2023
@C4-Staff C4-Staff reopened this Dec 4, 2023
@C4-Staff C4-Staff added the Q-18 label Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-313 grade-b Q-18 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

4 participants