sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

tsvetanovv - `addBlackList()` can be frontrunned #66

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

tsvetanovv

medium

addBlackList() can be frontrunned

Summary

In ERC20Token.sol we have addBlackList() function:

    function addBlackList(address evilUser) public onlyGuardian { 
        require(evilUser != address(0), "Invalid address");
        isBlackListed[evilUser] = true;
        emit AddedBlackList(evilUser);
    }

This function add suspicious account to the blacklist.

But addBlackList() is susceptible to front-running attacks because the state of the Ethereum mempool is public.

Vulnerability Detail

Attackers can listen to the mempool and front-run the addBlackList() function to transfer assets in advance before being blacklisted.

Impact

Attackers can bypass the blacklist mechanism.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/ERC20Token.sol#L259-L263

Tool used

Manual Review

Recommendation

Always execute addBlackList() function through a private mempool.

cvetanovv commented 1 year ago

Escalate for 10 USDC

This is a valid Medium issue. On previous Sherlock audits, this case is valid.

https://github.com/sherlock-audit/2023-02-telcoin-judging/issues/43

sherlock-admin commented 1 year ago

Escalate for 10 USDC

This is a valid Medium issue. On previous Sherlock audits, this case is valid.

https://github.com/sherlock-audit/2023-02-telcoin-judging/issues/43

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

ctf-sec commented 1 year ago

Valid low, I think it is normal to design add blocklist / remove blocklist / pause and unpause + the frontrunning can be avoided using flashloan

cvetanovv commented 1 year ago

Ok but then why was it valid in a previous contest? So it becomes very selective when it is valid and when it is not.

0xruhum commented 1 year ago

That could've been a mistake. There's no way you can prevent this from happening on the smart contract level. That's simply how Ethereum works. If there would be a way to limit it through changes to the protocol it would have been a valid issue.

ctf-sec commented 1 year ago

There's no way you can prevent this from happening on the smart contract level. That's simply how Ethereum works. If there would be a way to limit it through changes to the protocol it would have been a valid issue.

agree

jacksanford1 commented 1 year ago

Impact does not describe a material loss of funds for users. It's hard to imagine how frontrunning the blacklist function would cause a loss for users, so this will be invalid unless @cvetanovv further describes the loss of funds scenario.

hrishibhat commented 1 year ago

Result: Low Unique

sherlock-admin commented 1 year ago

Escalations have been resolved successfully!

Escalation status: