code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

ETH Deposit Handling Failure in `PrincipalToken.sol` #35

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

The PrincipalToken contract's deposit function is intended to handle ERC-20 token transactions but does not support the deposit of native ETH. Users attempting to deposit native ETH will experience transaction failures, leading to a poor user experience and potential loss of gas fees. Additionally, the inability to deposit native ETH limits the contract's functionality and accessibility, as users must first convert their ETH to the accepted ERC-20 token format before interacting with the contract.

Proof of Concept

for example PrincipalToken contract takes stETH as IBT

A user attempts to deposit native ETH by calling the deposit function and sending ETH along with the transaction.

        function deposit(
        uint256 assets,
        address ptReceiver,
        address ytReceiver
    ) public override returns (uint256 shares) {
        IERC20(_asset).safeTransferFrom(msg.sender, address(this), assets);
        IERC20(_asset).safeIncreaseAllowance(ibt, assets);
        uint256 ibts = IERC4626(ibt).deposit(assets, address(this));
        shares = _depositIBT(ibts, ptReceiver, ytReceiver);
    }

The transaction reverts because the deposit function is not marked as payable and thus cannot accept native ETH.

Even if the function were payable, the logic within the function expects to interact with an ERC-20 token, using methods like safeTransferFrom and safeIncreaseAllowance, which are not applicable to native ETH.

Since stETH's underlying asset is ETH, users might reasonably expect to be able to deposit ETH directly. The contract's inability to handle this case is a significant usability issue.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a new depositETH and WithdrawEth function that is marked as payable to accept native ETH.

Assessed type

Error

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

gzeon-c4 commented 8 months ago

feature request

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid