hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

Audit competition repository for Euro-Dollar (0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd)
https://hats.finance
MIT License
3 stars 2 forks source link

Malicious BLACKLISTER_ROLE can temporarily block burning mechanism blacklisting address(0) #84

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

Github username: @itsabinashb Twitter username: itsabinashb Submission hash (on-chain): 0xeafcb7a625760ac3151eb9bf15ec8b6114d11ffcd10320e28413713fe1b5821d Severity: medium

Description:

Description

A malicious BLACKLISTER_ROLE can temporarily DoS the burning mechanism of USDE i.e EuroDollar by blacklisting address(0). This occrurs due to logic of isValid(). The function says:

So, we know that during _burn() call token is transferred to address(0). As the USDE::_update() was modified where the function checking if the tx is valid or not using Validator::isValid(), here it will return false if address(0) is blacklisted i.e the burn will fail.

POC

Hacek00 commented 2 weeks ago

By doing this the malicious BLACKLISTER_ROLE can also DoS the deposit to the vault.

Hacek00 commented 2 weeks ago

@AndreiMVP

AndreiMVP commented 2 weeks ago

We do put trust in the BLACKLISTER with this, but I don't think it's more than it inherently has. If BLACKLISTER already has the power to ban users and restrict the token interaction to burning only, relatively I don't think banning address(0) is doing more damage.

Hacek00 commented 2 weeks ago

We do put trust in the BLACKLISTER with this, but I don't think it's more than it inherently has. If BLACKLISTER already has the power to ban users and restrict the token interaction to burning only, relatively I don't think banning address(0) is doing more damage.

To blacklist a user the BLACKLISTER_ROLE must have strong reason behind it & other Roles will be aware it, but if the BLACKLISTER_ROLE is malicious then he can simply blacklist the address(0) , by doing this he can INDIRECTLY DOSing the deposit & burn.

And also if he want to blacklist a single user he needs to do it one by one, if he want to blacklist 10 user then he need to send 10 tx but by blacklisting address(0) with 1 tx he can temporarily revert deposits & burns for all users as long as possible until it is catched.

AndreiMVP commented 2 weeks ago

I get what you say but I don't think we're running into an issue in practice here. As I say, there's no more risk than it inherently has.

To blacklist a user the BLACKLISTER_ROLE must have strong reason behind it & other Roles will be aware it

I don't understand this point. If BLACKLISTER_ROLE is malicious it doesn't need a reason to act maliciously

And also if he want to blacklist a single user he needs to do it one by one, if he want to blacklist 10 user then he need to send 10 tx

Not a valid concern here. He can submit a list of users or multiple txs. We don't consider gas fees in this context.

Hacek00 commented 2 weeks ago

To blacklist a user the BLACKLISTER_ROLE must have strong reason behind it & other Roles will be aware it

By this I wanted to say if a normal BLACKLISTER_ROLE want to blacklist an address then the address might be harmful for the protocol i.e there is a reason behind blacklisting it.

My whole point is, it will be best practice if address(0) is banned for blacklisting, it will eliminate any of described problem in future.
May not be a bug but an improvement suggestion. Can be considered as low severity.

@AndreiMVP