sherlock-audit / 2023-01-illuminate-judging

1 stars 0 forks source link

ayeslick - Depositors can lose their entire deposit #32

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

ayeslick

high

Depositors can lose their entire deposit

Summary

If autoRedeem is called after a market matures but before any calls to a redeem function and the addresses passed into the autoRedeem function approved the redeemer contract to move their funds, those customers will lose their entire principal token balance.

Vulnerability Detail

There's the potential for time to pass between a market maturing and someone calling autoRedeem. When autoRedeem is called there aren't any checks to make sure that the holdings mapping value is greater than zero. If a call to autoRedeem were to happen when the holdings mapping is zero, redeemed would be zero

uint256 redeemed = (amount * holdings[u][m]) / pt.totalSupply();

and

uint256 amount = pt.balanceOf(f[i]);
...
pt.authBurn(f[i], amount);

the total principal token balance of each address passed in would be burned without the addresses receiving any underlying due to them.

Impact

Customers' entire principal token balance is burned without them receiving any underlying

Code Snippet

https://github.com/sherlock-audit/2023-01-illuminate/blob/main/src/Redeemer.sol#L613 https://github.com/sherlock-audit/2023-01-illuminate/blob/main/src/Redeemer.sol#L616 https://github.com/sherlock-audit/2023-01-illuminate/blob/main/src/Redeemer.sol#L627

Tool used

Manual Review

Recommendation

Revert the transaction if the holdings mapping is empty

Duplicate of #19

IllIllI000 commented 1 year ago

This issue and https://github.com/sherlock-audit/2023-01-illuminate-judging/issues/19 seem like they should have the same severity

sourabhmarathe commented 1 year ago

Duplicate of #19.