Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Submission Details
Severity: medium
Invalid

The redemption function should confirm that the depositor's asset tokens were burned before sending underlying tokens back to them. The burn could fail silently.

Summary

The redemption function calculates how many underlying tokens a depositor is owed in exchange for the asset token shares they are redeeming, then burns those asset tokens and sends the underlying tokens to the depositor. But there is no check that the asset tokens were burned before the underlying tokens are sent back. The burn function could silently fail and then the depositor could receive their underlying tokens.

Vulnerability Details

function redeem(
IERC20 token,
uint256 amountOfAssetToken
) external revertIfZero(amountOfAssetToken) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
if (amountOfAssetToken == type(uint256).max) {
amountOfAssetToken = assetToken.balanceOf(msg.sender);
}
uint256 amountUnderlying = (amountOfAssetToken * exchangeRate) /
assetToken.EXCHANGE_RATE_PRECISION();
emit Redeemed(msg.sender, token, amountOfAssetToken, amountUnderlying);
assetToken.burn(msg.sender, amountOfAssetToken);
assetToken.transferUnderlyingTo(msg.sender, amountUnderlying);
}

Impact

A depositor could get their deposit plus their portion of accrued fees without having their asset token shares burned.

Tools Used

Manual review

Recommendations

Add a check that the burn was successful and revert the redemption if it was not:

bool success = assetToken.burn(msg.sender, amountOfAssetToken);
if(!success) {
revert ThunderLoan__BurnNotSuccessful()};
assetToken.transferUnderlyingTo(msg.sender, amountUnderlying);
}
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.