BoRonGod - No sllippage protection in addPrincipalToCommitmentGroup and burnSharesToWithdrawEarnings
Closed this issue · 0 comments
BoRonGod
high
No sllippage protection in addPrincipalToCommitmentGroup and burnSharesToWithdrawEarnings
Summary
No sllippage protection in addPrincipalToCommitmentGroup and burnSharesToWithdrawEarnings, which create great loss to users.
Vulnerability Detail
Attackers can frontrun addPrincipalToCommitmentGroup and burnSharesToWithdrawEarnings to create bad rate for innocent users.
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());
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()
);
totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;
principalToken.transfer(_recipient, principalTokenValueToWithdraw);
return principalTokenValueToWithdraw;
}
Impact
When a user addPrincipalToCommitmentGroup, if the contract experiences a profit (through liquidation) while their transaction is pending (waiting in the mempool), they will receive fewer shares than they expected.
When a user burnSharesToWithdrawEarnings, if the contract experiences a loss (through liquidation) while their transaction is pending (waiting in the mempool), they will receive fewer shares than they expected.
Code Snippet
Tool used
Manual Review
Recommendation
Both addPrincipalToCommitmentGroup/burnSharesToWithdrawEarnings functions should include slippage protection parameters provided by the users.
Duplicate of #141