The function burnSharesToWithdrawEarnings() is subject to Re-entrancy Vulnerability
Summary
In the contract LenderCommitmentGroup_Smart.sol, the function burnSharesToWithdrawEarnings transfers the funds before updating the state. This order of execution exposes the function, and subsequently the contract, to re-entrancy attacks.
Vulnerability Detail
In a re-entrancy attack, a malicious contract can call back into the vulnerable contract before the latter had a chance to finish its execution.
Impact
This potentially allows the attacker to drain funds from the contract.
To prevent re-entrancy attack, the state should be updated before calling another contract and transferring any assets. Using the "checks-effects-interactions" pattern can mitigate this issue.
So the recommended change to the code is as follows:
function burnSharesToWithdrawEarnings(
uint256 _amountPoolSharesTokens,
address _recipient
) external returns (uint256) {
poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);
uint256 principalTokenValueToWithdraw = _valueOfUnderlying(
_amountPoolSharesTokens,
sharesExchangeRateInverse()
);
totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;
//Ensure the state (balance) is updated before transfer is made
//This prevents re-entrancy attacks
uint256 newBalance = totalPrincipalTokensWithdrawn;
require(
newBalance <= address(this).balance,
"Insufficient balance in contract"
);
principalToken.transfer(_recipient, principalTokenValueToWithdraw);
return newBalance;
}
w42d3n
medium
The function burnSharesToWithdrawEarnings() is subject to Re-entrancy Vulnerability
Summary
In the contract LenderCommitmentGroup_Smart.sol, the function
burnSharesToWithdrawEarnings
transfers the funds before updating the state. This order of execution exposes the function, and subsequently the contract, to re-entrancy attacks.Vulnerability Detail
In a re-entrancy attack, a malicious contract can call back into the vulnerable contract before the latter had a chance to finish its execution.
Impact
This potentially allows the attacker to drain funds from the contract.
Code Snippet
https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L396-L415
Tool used
Manual Review
Recommendation
To prevent re-entrancy attack, the state should be updated before calling another contract and transferring any assets. Using the "checks-effects-interactions" pattern can mitigate this issue. So the recommended change to the code is as follows:
Duplicate of #179