kennedy1030 - Invalid check in `LenderCommitmentGroup_Smart.burnSharesToWithdrawEarnings()`.
Closed this issue · 1 comments
kennedy1030
high
Invalid check in LenderCommitmentGroup_Smart.burnSharesToWithdrawEarnings()
.
Summary
Without any other interruption, a malicious user can break the LenderCommitmentGroup_Smart
by conducting a series of actions: deposit, borrow, liquidate, donation, and withdraw.
Vulnerability Detail
Let's consider the following scenario:
- Bob deposits 100 wei to an empty
LenderCommitmentGroup_Smart
.- state: Bob's share = 100, totalPrincipalTokensCommitted = 100, totalSupply = 100.
- Bob borrows 80 wei with no duration.
- Bob liquidates his loan at the time
_loanDefaultedTimestamp + 86400 + 5000
.- state: tokenDifferenceFromLiquidations = -40.
- Bob donates 60 wei to the
LenderCommitmentGroup_Smart
. - Bob withdraws 99 shares.
- state: totalPrincipalTokensWithdrawn = 60, totalSupply = 1.
Finally, the exchange rate will be (100 - 40 - 60) / 1 = 0
, leading to breaking the LenderCommitmentGroup_Smart
.
After that, if someone deposits, he will receive no share and Bob can take all of that.
This problem occurs because there is no balance check in withdrawing.
Impact
Without anyother's interruption, a malicious user can break the LenderCommitmentGroup_Smart
.
Code Snippet
Tool used
Manual Review
Recommendation
Deposit and withdrawal should be improved as follows.
function addPrincipalToCommitmentGroup(
uint256 _amount,
address _sharesRecipient
) external returns (uint256 sharesAmount_) {
//transfers the primary principal token from msg.sender into this contract escrow
principalToken.transferFrom(msg.sender, address(this), _amount);
sharesAmount_ = _valueOfUnderlying(_amount, sharesExchangeRate());
+ require(sharesAmount > 0);
totalPrincipalTokensCommitted += _amount;
//principalTokensCommittedByLender[msg.sender] += _amount;
//mint shares equal to _amount and give them to the shares recipient !!!
poolSharesToken.mint(_sharesRecipient, sharesAmount_);
}
function burnSharesToWithdrawEarnings(
uint256 _amountPoolSharesTokens,
address _recipient
) external returns (uint256) {
poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);
uint256 principalTokenValueToWithdraw = _valueOfUnderlying(
_amountPoolSharesTokens,
sharesExchangeRateInverse()
);
+ int256 balanceOfPool = int256(totalPrincipalTokensCommitted) - int256(totalPrincipalTokensWithdrawn)
+ + int256(totalInterestCollected) + int256(tokenDifferenceFromLiquidations)
+ - int256(totalPrincipalTokensLended) + int256(totalPrincipalTokensRepaid);
+ require(balanceOfPool > 0 && principalTokenValueToWithdraw > 0);
+ require(principalTokenValueToWithdraw <= uint256(balanceOfPool));
totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;
principalToken.transfer(_recipient, principalTokenValueToWithdraw);
return principalTokenValueToWithdraw;
}
Invalid, direct donation is not possible to influence shares exchange rate since pool value used to calculate rate does not include principle token balance for calculations to determine principalTokenValueToWithdraw
.