Owner can be attacked by a malicious contract when the owner calls withdraw, thereby draining the users assets.

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L285-L286

Vulnerability details

Impact

Report Details:

In the withdraw function, there is a potential vulnerability related to reentrancy. The IERC4626(ibt).withdraw call is executed before an essential state is set, potentially exposing the contract to reentrancy attacks. To mitigate this risk and enhance security, consider introducing a reentrancy guide in the withdraw function.

Here is the existing code snippet for reference:

function withdraw(
    uint256 assets,
    address receiver,
    address owner
) public override returns (uint256 shares) {
    _beforeWithdraw(assets, owner);
    (uint256 _ptRate, uint256 _ibtRate) = _getPTandIBTRates(false);
    uint256 ibts = IERC4626(ibt).withdraw(assets, receiver, address(this));
    shares = _withdrawShares(ibts, receiver, owner, _ptRate, _ibtRate);
}

Vulnerability Summary:

The current contract design exhibits a vulnerability where a malicious receiver contract can exploit the withdrawal mechanism, allowing an attacker to repeatedly call the withdrawal function, draining the asset balance and shares of the legitimate owner.

Exploitation Scenario:

1. Initiation of Withdrawal:

   • The legitimate owner initiates a withdrawal, sending a specified amount of assets to a receiver contract.

2. Trust in Receiver Contract:

   • The contract places significant trust in the receiver contract, assuming that it will follow the intended behavior.

3. Reentrancy Exploitation:

   • The malicious receiver contract can exploit this trust by re-calling the withdrawal function within its own execution context.

4. Bypassing Safeguards:

   • The _beforeWithdraw hook, responsible for pre-withdrawal checks, validates the owner, checks the time, updates yield, and ensures sufficient assets. However, these checks do not prevent the reentrant exploitation.

5. Rate Retrieval:

   • The contract fetches the PT and IBT rates (_getPTandIBTRates) as part of the withdrawal process. Unfortunately, this step does not mitigate the attack.

6. Repeat Withdrawals:

   • The attacker can repeatedly call the withdrawal function until the asset balance is depleted, leading to an unintended transfer of all shares and assets.

Impact:

The exploitation of this vulnerability results in a scenario where the legitimate owner's shares and assets are drained by the malicious receiver, causing substantial financial loss.

Explanation:

1. Vulnerability Concern:

   • The IERC4626(ibt).withdraw call is made before executing important state changes within the _withdrawShares function.

2. Reentrancy Risk:

   • Executing external calls before setting crucial states opens the possibility of reentrancy attacks, where an external contract could manipulate the contract's state during the execution of the withdraw function.

Tools Used

Manual code reading

Recommended Mitigation Steps

Enhancing Security: Introducing Reentrancy Safeguards in the Withdraw Function

• Consider introducing a reentrancy guide at the beginning of the withdraw function to ensure that all necessary states are set before interacting with external contracts.

By adopting this practice, you add an extra layer of security to the withdraw function, reducing the risk of potential reentrancy attacks. This adjustment aligns with best practices in smart contract development and contributes to a more robust and secure system.

Conclusion:

Addressing these issues will significantly improve the security of the withdrawal mechanism, protecting users from potential financial exploitation and ensuring the integrity of the contract's intended functionality.

Assessed type

Reentrancy

[c4-pre-sort]

gzeon-c4 marked the issue as insufficient quality report

[gzeoneth]

invalid

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid