search

sherlock-audit / 2024-02-smilee-finance-judging

2 stars 1 forks source link

Chad0 - Inadequate checking on the arbitrary `receiver` address given to the external `deposit` function introduces compliance risks of contaminating the protocol & users' addresses #114

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 9 months ago

Chad0

high

Inadequate checking on the arbitrary receiver address given to the external deposit function introduces compliance risks of contaminating the protocol & users' addresses

Summary

Inadequate checking on the arbitrary receiver address given to the external deposit function introduces compliance risks of contaminating the protocol & users' addresses.

Vulnerability Detail

In Line#324 of the Vault.sol, the contract has an external deposit function which is meant to let the users deposit their baseToken (USDC for example) into the vault. This function is designed to allow any user to specify any arbitrary address as the receiver, so that this receiver address becomes the beneficiary and can receive shares, and can later interact with the protocol to redeem shares, to withdraw fund...etc.

However, in the function there is not adequate checking performed on this arbitrary receiver input, so it can introduce various types of risks to the protocol. To name a few:

  1. A malicious user can specify an address of his which has been sanctioned/blacklisted by the baseToken protocols like USDC or USDT as the receiver, hence creating frequent downstream interactions from such a bad address with the Vault address. It can lead to the Vault address being contaminated and also added to the blacklists of USDC/USDT..etc. What's worse, if the top or most frequent users of Vault also got their addresses blacklisted as a chain of contamination, then that would be a catastrophic situation. Such an attack is easy to implement, and can possibly grow into a quite severe impact on the protocol and the users, so I deem the severity of this issue as HIGH.
  2. A genuine user may also make a mistake by not initializing the address of receiver before they use their contracts to interact with the Vault contract, so that the address(0) is accidentally used as the receiver.

Impact

  1. Risks of getting blacklisted for USDC/USDT are brought to the protocol, and maybe to the most frequent users of the protocol.
  2. Risks of users losing fund to address(0).

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/Vault.sol#L324

Tool used

Manual Review

Recommendation

  1. Added checking on the receiver that, revert the trxn if the return is true from the isBlacklisted function of the baseToken (USDC, USDT...etc.)
  2. Also, revert the trxn if the receiver is address(0).
sherlock-admin3 commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, this is user mistake and external (e.g. USDC) blacklisted addresses issues are invalid in sherlock

takarez commented:

invalid