code-423n4 / 2023-10-badger-findings

1 stars 1 forks source link

Not able to make position healthy when stETH collateral is paused #296

Closed c4-submissions closed 9 months ago

c4-submissions commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/BorrowerOperations.sol#L783

Vulnerability details

Impact

Not able to make position healthy when stETH collateral is paused

Proof of Concept

In BorrowOperation, the user and protocol relies on this function to move the collateral from the user to activePool

function _activePoolAddColl(uint256 _stEthBalance, uint256 _sharesToTrack) internal {
        // NOTE: No need for safe transfer if the collateral asset is standard. Make sure this is the case!
        collateral.transferFrom(msg.sender, address(activePool), _stEthBalance);
        activePool.increaseSystemCollShares(_sharesToTrack);
    }

the user replies this funciton to add more collateral to make account healthy

however,

if we take a look at the stETH smart contract

https://etherscan.io/token/0xae7ab96520de3a18e5e111b5eaab095312d7fe84#readProxyContract

the implementation contract is https://etherscan.io/address/0x17144556fd3424edc8fc8a4c940b2d04936d17eb#code

when the admin of stETH paused the token, no transfer can be allowed and no flashloan can be borrowed

https://etherscan.io/address/0x17144556fd3424edc8fc8a4c940b2d04936d17eb#code#F27#L445

 function _transferShares(address _sender, address _recipient, uint256 _sharesAmount) internal {
        require(_sender != address(0), "TRANSFER_FROM_ZERO_ADDR");
        require(_recipient != address(0), "TRANSFER_TO_ZERO_ADDR");
        require(_recipient != address(this), "TRANSFER_TO_STETH_CONTRACT");
        _whenNotStopped();

        uint256 currentSenderShares = shares[_sender];
        require(_sharesAmount <= currentSenderShares, "BALANCE_EXCEEDED");

        shares[_sender] = currentSenderShares.sub(_sharesAmount);
        shares[_recipient] = shares[_recipient].add(_sharesAmount);
    }

the admin on stETh can pause the contract to block collateral transfer

in this case, the interest still accuring while the user cannot make account more healthy by adding more collateral

in case when the stETH price drops, the user has no option, their position is subject to liquidation

Tools Used

Manual Review

Recommended Mitigation Steps

make sure interest is not accuring when the collateral token transfer is paused, and detect when the stETH unpause, leave sufficient time for user to add more collateral

Assessed type

Timing

bytes032 commented 9 months ago

CleanShot 2023-11-15 at 12  42 45 OOS

c4-pre-sort commented 9 months ago

bytes032 marked the issue as insufficient quality report

c4-pre-sort commented 9 months ago

bytes032 marked the issue as primary issue

c4-sponsor commented 9 months ago

GalloDaSballo (sponsor) disputed

c4-judge commented 9 months ago

jhsagd76 marked the issue as unsatisfactory: Out of scope