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

SilentDefendersOfDeFi - Auction can be forcefully paused #125

Closed
sherlock-admin2 opened this issue Dec 1, 2023 · 0 comments
Closed

SilentDefendersOfDeFi - Auction can be forcefully paused #125

sherlock-admin2 opened this issue Dec 1, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Dec 1, 2023

SilentDefendersOfDeFi

medium

Auction can be forcefully paused

Summary

According to the EIP-150 call opcode can consume as most 63/64 of parent calls' gas. That means token.mint() can fail since there will be no gas.

This will pause the auction, which has to be restarted by governance.

Vulnerability Detail

Following issue was reported in the previous audit, which was later determined as invalid:
code-423n4/2022-09-nouns-builder-findings#719

The issue describes the possibility of mint reverting due to lack of gas and causing the contract to be paused.
This is possible because mint() can mint up to 100 tokens in one call, consuming more then 63/64 of parent contracts calls gas.
The issue was later invalidated because in the old version the contract was catching only string errors, which does not include out of gas.

The current version does not only catch string errors:

       } catch {
            // Pause the contract if token minting failed
            _pause();
            return false;
        }

This makes the before described attack possible.
Following POC is slightly modified from the POC provided by alex in the old issue:

https://gist.github.com/Shogoki/9c98564ed1d2e29b5bd1bbc95e43a359

Impact

Forcefully pause DAOs that have large amount of founders.

Code Snippet

https://github.com/sherlock-audit/2023-09-nounsbuilder/blob/main/nouns-protocol/src/auction/Auction.sol#L324

Tool used

Manual Review

Recommendation

Catch only string errors / add a check for gasLeft(); at the start of createAuction();

Duplicate of #243

@github-actions github-actions bot closed this as completed Dec 6, 2023
@github-actions github-actions bot added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Dec 6, 2023
@sherlock-admin sherlock-admin changed the title Precise Ginger Meerkat - Auction can be forcefully paused SilentDefendersOfDeFi - Auction can be forcefully paused Dec 13, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Dec 13, 2023
@Czar102 Czar102 added the Medium A valid Medium severity issue label Dec 21, 2023
@sherlock-admin sherlock-admin added Reward A payout will be made for this issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label and removed Non-Reward This issue will not receive a payout labels Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants