sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

__141345__ - `autoRedeem()` could be abused to dilute other users fund #210

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

141345

high

autoRedeem() could be abused to dilute other users fund

Summary

The underlying amount users gets by autoRedeem() is based on the PT share percentage, this mechanism could be abused by malicious user with some vector like sandwich attack.

Vulnerability Detail

Imagine the following:

  1. Alice has balance of 1,000 PT in the beginning, 10% of the 10,000 total supply of PT. holdings[u1][m1] is $1,000. In principle the amount Alice deserves is $100
  2. a malicious user firstly buy 90,000 PT, inflate the PT total supply to 100,000. Now alice only has 1% of the total supply
  3. the malicious user calls autoRedeem() for Alice, the 1000 PT balance is burned, and the underlying redeemed is only $10 for Alice.
  4. the malicious user redeems the 90,000 PT for another (u2, m2), which is not affected by the total supply inflation.
  5. At the end Alice loses some fund due to the diluted share.

Actually, the malicious user can also redeem in the same m(u1, m1), because the majority of the holdings[u1][m1] cold be taken.

The key point is the PT share percentage can be easily manipulated.

Impact

User's PT balance share could be manipulated, some users will lose fund.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L517

Tool used

Manual Review

Recommendation

Do the underlying redeem amount calculation based on the PT balance, instead of the external markets redeem amount and PT share percentage.

sourabhmarathe commented 1 year ago

A malicious user would not be able to call autoRedeem on behalf of Alice due to the fact that Alice must approve the usage of autoRedeem in advance. In addition, purchasing PTs does not inflate the supply of them. Instead, a user must mint PTs in order to create them.

JTraversa commented 1 year ago

To reiterate,

a malicious user firstly buy 90,000 PT, inflate the PT total supply to 100,000.

This would not work as suggested as the total supply itself is dependent on external principal token deposits, meaning the attack would not end up stealing anything from the user.

141345 commented 1 year ago

Escalate for 21 USDC

Alice must approve the usage of autoRedeem in advance.

Yes, the issue here is discussing the users who choose to approve autoRedeem usage. And it seems to be the rationale behind this function, let some users have hands-off operation.

In addition, purchasing PTs does not inflate the supply of them. Instead, a user must mint PTs in order to create them.

This would not work as suggested as the total supply itself is dependent on external principal token deposits, meaning the attack would not end up stealing anything from the user.

Here "purchasing" just mean mint(), because I treat transferFrom() the necessary principal token as purchasing.

https://github.com/sherlock-audit/2022-10-illuminate/blob/2daab8d35e2362c272a62235ec135bc01e0fb79d/src/Lender.sol#L270-L288

When this mint() call authMint(), new PTs will increase the totalSupply, as a result, Alice's share in total PTs will decrease.

The point of this issue here is, the user's redemption amount depends on the shares of the total PTs, which can be artificially influenced.

sherlock-admin commented 1 year ago

Escalate for 21 USDC

Alice must approve the usage of autoRedeem in advance.

Yes, the issue here is discussing the users who choose to approve autoRedeem usage. And it seems to be the rationale behind this function, let some users have hands-off operation.

In addition, purchasing PTs does not inflate the supply of them. Instead, a user must mint PTs in order to create them.

This would not work as suggested as the total supply itself is dependent on external principal token deposits, meaning the attack would not end up stealing anything from the user.

Here "purchasing" just mean mint(), because I treat transferFrom() the necessary principal token as purchasing.

https://github.com/sherlock-audit/2022-10-illuminate/blob/2daab8d35e2362c272a62235ec135bc01e0fb79d/src/Lender.sol#L270-L288

When this mint() call authMint(), new PTs will increase the totalSupply, as a result, Alice's share in total PTs will decrease.

The point of this issue here is, the user's redemption amount depends on the shares of the total PTs, which can be artificially influenced.

You've created a valid escalation for 21 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.