-
Notifications
You must be signed in to change notification settings - Fork 0
hyh - External PT redeem functions can be reentered to double count the received underlying funds #236
Comments
You've created a valid escalation for 50 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. |
Escalation accepted |
This issue's escalations have been accepted! Contestants' payouts and scores will be updated according to the changes made on this issue. |
hyh
high
External PT redeem functions can be reentered to double count the received underlying funds
Summary
There are two versions of external PT redeem() functions in Redeemer: multi-PT one and Sense. Both calculated underlying funds returned from redeem as a balance difference, both can be reentered, and Sense one calls user-supplied adapter to perform the redeem.
An attacker can create a pre-cooked contract that first calls redeem() for all other types of PT, then proceeds to call the correct Sense adapter. All the funds obtained from PT except Sense will be double counted. The attacker will now need to burn its Illuminate PTs first, obtaining the major part of the underlying funds and making the contract insolvent for all other users.
Vulnerability Detail
Bob the attacker will:
Lend 1 block before maturity, wait 1 block, then run the following with elevated gas cost
Create wrapper 'a', that do call Sense pool, but before that calls multi-PT redeem with all PTs available on Lender's balance the corresponding number of times
Call correct Sense adapter to redeem. Result is that all besides Sense is double counted in
holdings
Be first to redeem Illuminate PTs, obtaining up to double amount of the funds
Other users will soon be unable to redeem due to the lack of underlying funds in the contract
Impact
Bob will be able to extract the most part of the total underlying funds, making the contract up to insolvent for all other users.
It can be done without any additional requirements, so setting the severity to be high.
Code Snippet
Swivel, Yield, Element, Pendle, APWine, Tempus and Notional (multi-PT) redeem() isn't protected from reentracy:
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L209-L333
Sense redeem() records
IERC20(u).balanceOf(address(this)) - starting
as recovered underlying funds, calling user-supplied adaptera
in-between balance snapshots:https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L335-L398
Bob will supply
a
which first call multi-PT version of redeem() above with all available types of PTs, maximizing the underlying output, then calls the correct Sense adapter to obtain the Sense part. All underlying funds from all other types of PTs will be counted twice, first in the multi-PT redeem, second in Sense redeem.Tool used
Manual Review
Recommendation
If there is no desire to refactor Sense support from the current version, that calls user-supplied contract, consider adding reentrancy guard modifiers to both redeem() functions.
Illuminate redeem modifies the
holdings
in the opposite direction, but for the sake of reducing the surface altogether consider adding reentrancy guard there as well. Additional cost is well justified in the both cases.The text was updated successfully, but these errors were encountered: