sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

wangxx2026 - Hackers can obtain users' lend and collateral assets by helping users liquidate #45

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

wangxx2026

high

Hackers can obtain users' lend and collateral assets by helping users liquidate

Summary

If the assets borrowed by the user are much less than the assets of the lend and collateral, the hacker can help the user pay off the loan, thereby obtaining the user's lend and collateral assets. In the end hackers are profitable

Vulnerability Detail

The user calls the liquidate method and needs to repay the debt to continue, as shown in the code below ---->1.

After the debt is repaid, the corresponding proportion of collateralSize and underlyingVaultShare will be transferred to msg.sender. Note that msg.sender here can be anyone, and there is no judgment whether pos.owner is equal to msg.sender, so any user can operate. The code is shown as --->2, 3, 4.

For example, the user's underlyingVaultShare asset is 100$, and the collateralSize is 100$, but the borrowed asset is only 1$. Users can get 200$ by repaying 1$.And this happens all the time because borrowing is always less than collateral

The information on the blockchain is public, and it is not difficult to obtain the user's position.

    function liquidate(
        uint256 positionId,
        address debtToken,
        uint256 amountCall
    ) external override lock poke(debtToken) {
   ...
        /// Repay the debt and get details of repayment.
        uint256 oldShare = pos.debtShare;
       (uint256 amountPaid, uint256 share) = _repay(     // ---->1    
            positionId,
            debtToken,
            amountCall
        );

        /// Calculate the size of collateral and underlying vault share that the liquidator will get.
        uint256 liqSize = (pos.collateralSize * share) / oldShare;
        uint256 uVaultShare = (pos.underlyingVaultShare * share) / oldShare;

        /// Adjust the position's debt and collateral after liquidation.
        pos.collateralSize -= liqSize;
        pos.underlyingVaultShare -= uVaultShare;

        /// Transfer the liquidated collateral (Wrapped LP Tokens) to the liquidator.
        IERC1155Upgradeable(pos.collToken).safeTransferFrom(
            address(this),
            msg.sender,                       // ---->2
            pos.collId,
            liqSize,
            ""
        );
        /// Transfer underlying collaterals(vault share tokens) to liquidator
        if (_isSoftVault(pos.underlyingToken)) {
            IERC20Upgradeable(bank.softVault).safeTransfer(
               msg.sender,                    // ---->3 
                uVaultShare
            );
        } else {
            IERC1155Upgradeable(bank.hardVault).safeTransferFrom(
                address(this),
                msg.sender,                         // ---->4
                uint256(uint160(pos.underlyingToken)),
                uVaultShare,
                ""
            );
        }
...
    }

Impact

Acquire users' assets at a relatively low cost

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/7c7e1c4a8f3012d1afd2e598b656010bb9127836/blueberry-core/contracts/BlueBerryBank.sol#L531-L535

https://github.com/sherlock-audit/2023-07-blueberry/blob/7c7e1c4a8f3012d1afd2e598b656010bb9127836/blueberry-core/contracts/BlueBerryBank.sol#L544-L621

Tool used

Manual Review

Recommendation

Add

 require(msg.sender==pos.owner, "only pos owner");

restriction

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

shogoki commented:

Liquidation is intended to work like this

0xyPhilic commented:

invalid because liquidations are supposed to be open for anyone to execute

Kral01 commented:

thats how liquidations work