pkqs90 - In `LenderCommitmentGroup_Smart.sol#burnSharesToWithdrawEarnings()`, shares are burned before `principalTokenValueToWithdraw` is calculated, causing users to withdraw more tokens than expected.
Closed this issue · 0 comments
pkqs90
high
In LenderCommitmentGroup_Smart.sol#burnSharesToWithdrawEarnings()
, shares are burned before principalTokenValueToWithdraw
is calculated, causing users to withdraw more tokens than expected.
Summary
While burning shares, the amount of tokens for withdraw should be burnedShares * totalAssets / totalShares
. However, the code burns shares before calculating the amount of withdrawn tokens, which means the totalShares
is decreased. This leads to users being able to withdraw more tokens than expected.
Vulnerability Detail
burnSharesToWithdrawEarnings()
is the function for users burning shares. The poolSharesToken.burn()
is performed before calculating principalTokenValueToWithdraw
, and we can see the poolSharesToken.totalSupply()
is used while calculating the asset to share ratio.
An example of the issue is: Current pool has 10000 principalTokens and 5000 shares, a user withdraws 2500 shares. The amount of principalTokens he is suppose to withdraw is 5000, however since the 2500 shares are burned before calculation, the real value is 2500 * 10000 / (5000 - 2500) = 10000
, and the user withdraws all of the principalTokens.
function burnSharesToWithdrawEarnings(
uint256 _amountPoolSharesTokens,
address _recipient
) external returns (uint256) {
> poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);
> uint256 principalTokenValueToWithdraw = _valueOfUnderlying(
> _amountPoolSharesTokens,
> sharesExchangeRateInverse()
);
totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;
principalToken.transfer(_recipient, principalTokenValueToWithdraw);
return principalTokenValueToWithdraw;
}
function _valueOfUnderlying(uint256 amount, uint256 rate)
internal
pure
returns (uint256 value_)
{
if (rate == 0) {
return 0;
}
value_ = (amount * EXCHANGE_RATE_EXPANSION_FACTOR) / rate;
}
function sharesExchangeRate() public view virtual returns (uint256 rate_) {
uint256 poolTotalEstimatedValue = getPoolTotalEstimatedValue();
if (poolSharesToken.totalSupply() == 0) {
return EXCHANGE_RATE_EXPANSION_FACTOR; // 1 to 1 for first swap
}
> rate_ =
> (poolTotalEstimatedValue *
> EXCHANGE_RATE_EXPANSION_FACTOR) /
> poolSharesToken.totalSupply();
}
function sharesExchangeRateInverse()
public
view
virtual
returns (uint256 rate_)
{
return
(EXCHANGE_RATE_EXPANSION_FACTOR * EXCHANGE_RATE_EXPANSION_FACTOR) /
sharesExchangeRate();
}
Impact
Users withdraw more tokens than they should.
Code Snippet
Tool used
Manual review
Recommendation
Burn the shares after calculating principalTokenValueToWithdraw
.
Duplicate of #19