search

sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

ak1 - ERC5095.sol - approval based `redeem` and `withdraw` will not be safe. #192

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

high

ERC5095.sol - approval based redeem and withdraw will not be safe.

Summary

Approval based redeem and withdraw functionalities will not be safe. The malicious approved address can transfer the fund either to his/her own account or any other account that they wish,

Vulnerability Detail

When look at the approval based withdraw and redeem,

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L246

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L272

the fund is sent to address r that mean the address r can be any address. either it can be msg.sender or any other address that is set by msg.sender.

The problem here is, the msg.sender can transfer the fund wither his/her own account or to the account that they want.

Impact

The malicious approved address can take out funds either to his/her account and to any other account that they wish.

imo, this will not be safe. actual owner will loose his/her control on their fund.

Code Snippet

withdraw - approval based.

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L228-L249

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L262-L275

Redeem - approval based

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L304-L316

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/tokens/ERC5095.sol#L330-L343

Tool used

Manual Review

Recommendation

It is suggested to transfer the fund to the owner address.

Owner can be allowed to handle his/her fund.

or, check whether the r is owner

aktech297 commented 1 year ago

Escalate for 2 USDC I believe this is valid issue and need to be revisited. I am escalating to recheck this one.

sherlock-admin commented 1 year ago

Escalate for 2 USDC I believe this is valid issue and need to be revisited. I am escalating to recheck this one.

You've created a valid escalation for 2 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

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