sherlock-audit / 2023-10-real-wagmi-judging

16 stars 14 forks source link

0xkazim - attacker can front-running the lender calls when `isEmergency` is true #169

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xkazim

medium

attacker can front-running the lender calls when isEmergency is true

Summary

This report focuses on a vulnerability within a lending protocol where lenders can withdraw their liquidity through the repay function during emergency situations when the isEmergency flag is true. However, a lack of checks for the position owner, specifically whether the caller is the lender, may expose a critical security flaw.

Vulnerability Detail

Consider a scenario where a lender intends to utilize the repay function to withdraw their assets:

 function repay(
        RepayParams calldata params,
        uint256 deadline
    ) external nonReentrant checkDeadline(deadline) {
        BorrowingInfo memory borrowing = borrowingsInfo[params.borrowingKey];
        // Check if the borrowing key is valid
        (borrowing.borrowedAmount == 0).revertError(ErrLib.ErrorCode.INVALID_BORROWING_KEY);

        bool zeroForSaleToken = borrowing.saleToken < borrowing.holdToken;
        uint256 liquidationBonus = borrowing.liquidationBonus;
        int256 collateralBalance;
        // Update token rate information and get holdTokenRateInfo storage reference
        (, TokenInfo storage holdTokenRateInfo) = _updateTokenRateInfo(
            borrowing.saleToken,
            borrowing.holdToken
        );
        {
            // Calculate collateral balance and validate caller
            uint256 accLoanRatePerSeconds = holdTokenRateInfo.accLoanRatePerSeconds;
            uint256 currentFees;
            (collateralBalance, currentFees) = _calculateCollateralBalance(
                borrowing.borrowedAmount,
                borrowing.accLoanRatePerSeconds,
                borrowing.dailyRateCollateralBalance,
                accLoanRatePerSeconds
            );

            (msg.sender != borrowing.borrower && collateralBalance >= 0).revertError(
                ErrLib.ErrorCode.INVALID_CALLER
            );

            // Calculate liquidation bonus and adjust fees owed

            if (
                collateralBalance > 0 &&
                (currentFees + borrowing.feesOwed) / Constants.COLLATERAL_BALANCE_PRECISION >
                Constants.MINIMUM_AMOUNT
            ) {
                liquidationBonus +=
                    uint256(collateralBalance) /
                    Constants.COLLATERAL_BALANCE_PRECISION;
            } else {
                currentFees = borrowing.dailyRateCollateralBalance;
            }

            // Calculate platform fees and adjust fees owed
            borrowing.feesOwed += _pickUpPlatformFees(borrowing.holdToken, currentFees);
        }
        // Check if it's an emergency repayment
        //@audit no check if msg.sender is owner of the position !
        if (params.isEmergency) {
            (collateralBalance >= 0).revertError(ErrLib.ErrorCode.FORBIDDEN);
            (
                uint256 removedAmt,
                uint256 feesAmt,
                bool completeRepayment
            ) = _calculateEmergencyLoanClosure(
                    zeroForSaleToken,
                    params.borrowingKey,
                    borrowing.feesOwed,
                    borrowing.borrowedAmount
                );
            (removedAmt == 0).revertError(ErrLib.ErrorCode.LIQUIDITY_IS_ZERO);
            // prevent overspent
            // Subtract the removed amount and fees from borrowedAmount and feesOwed
            borrowing.borrowedAmount -= removedAmt;
            borrowing.feesOwed -= feesAmt;
            feesAmt /= Constants.COLLATERAL_BALANCE_PRECISION;
            // Deduct the removed amount from totalBorrowed
            holdTokenRateInfo.totalBorrowed -= removedAmt;
            // If loansInfoLength is 0, remove the borrowing key from storage and get the liquidation bonus
            if (completeRepayment) {
                LoanInfo[] memory empty;
                _removeKeysAndClearStorage(borrowing.borrower, params.borrowingKey, empty);
                feesAmt += liquidationBonus;
            } else {
                BorrowingInfo storage borrowingStorage = borrowingsInfo[params.borrowingKey];
                borrowingStorage.dailyRateCollateralBalance = 0;
                borrowingStorage.feesOwed = borrowing.feesOwed;
                borrowingStorage.borrowedAmount = borrowing.borrowedAmount;
                // Calculate the updated accLoanRatePerSeconds
                borrowingStorage.accLoanRatePerSeconds =
                    holdTokenRateInfo.accLoanRatePerSeconds -
                    FullMath.mulDiv(
                        uint256(-collateralBalance),
                        Constants.BP,
                        borrowing.borrowedAmount // new amount
                    );
            }
        }

In the code snippet above, there is no validation to confirm whether the caller is the owner of the position when isEmergency is set to true. As a result, an attacker can exploit this by initiating a call to the repay function with isEmergency set to true, using the same parameters as the lender. The attacker can front-run the lender's transaction, gaining access to the removedAmt + feesAmt assets.

Impact

The impact of this vulnerability is severe, as an attacker can front-run a lender's transaction during emergency calls, effectively seizing the fees and the removed amount of assets

Code Snippet

https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L581-L626

Tool used

Manual Review

Recommendation

It is recommended to implement checks to verify the ownership of the position when isEmergency is set to true. Alternatively, consider using a different technique, such as hashing the parameter values, to enhance the security of the process.

Duplicate of #130