sherlock-audit / 2024-04-teller-finance-judging

10 stars 9 forks source link

blockchain555 - The owner of the `LenderGroup_Smart` contract can maliciously deploy a non-lending pool or set an unfair interest rate. #252

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

blockchain555

medium

The owner of the LenderGroup_Smart contract can maliciously deploy a non-lending pool or set an unfair interest rate.

Summary

When deploying a pool because the max cap of _interestRateLowerBound and _interestRateUpperBound in the LenderCommitmentGroup_Smart.sol#initialize() function is not specified, a pool that cannot be loaned can be deployed or an unfair interest rate can be set.

Vulnerability Detail

LenderGroup_Smart contract is a pool for lending principal tokens. This pool is deployed one for each token pair so that one poolShareToken is deployed for each token pair in Uniswap.

    function _deployPoolSharesToken()
        internal
        onlyInitializing
        returns (address poolSharesToken_)
    {

221:    require(
            address(poolSharesToken) == address(0),
            "Pool shares already deployed"
        );

227:    (string memory name, string memory symbol ) = _generateTokenNameAndSymbol(
228:        address(principalToken),
229:        address(collateralToken)
230:    );

232:    poolSharesToken = new LenderCommitmentGroupShares(
            name,
            symbol,
            18  
        );

        return address(poolSharesToken);
    }

And all parameters cannot be changed after they are set in the initialize() function because there are no functions that set the value of parameters.

    function initialize(
        address _principalTokenAddress,
        address _collateralTokenAddress,
        uint256 _marketId,
        uint32 _maxLoanDuration,
        uint16 _interestRateLowerBound,
        uint16 _interestRateUpperBound,
        uint16 _liquidityThresholdPercent, // When 100% , the entire pool can be drawn for lending.  When 80%, only 80% of the pool can be drawn for lending. 
        uint16 _collateralRatio, //the required overcollateralization ratio.  10000 is 1:1 baseline , typically this is above 10000
        uint24 _uniswapPoolFee,
        uint32 _twapInterval
    ) external initializer returns (address poolSharesToken_) {
        SNIP...

        maxLoanDuration = _maxLoanDuration;
197:    interestRateLowerBound = _interestRateLowerBound;
198:    interestRateUpperBound = _interestRateUpperBound;

203:    require(interestRateLowerBound <= interestRateUpperBound, "invalid _interestRateLowerBound");

        SNIP...
    }

However, as you can see, the max cap of interestRateLowerBound and interestRateUpperBound is not checked, but only the condition of #L203 is checked. Therefore, a malicious distributor can set these two variables to type(uint16).max to disable lending or intentionally set them high to receive unfair interest. 1.If these two variables are set to type(uint16).max, the borrower can only set APR to type(uint16).max. In this case, DOS occurs due to overflow of the V2Calculations.sol#calculatePaymentCycleAmount() function, and therefore the borrower cannot execute the submitBid() function.

V2Calculations.sol#calculatePaymentCycleAmount():

    _principal.percent(_apr).percent(
        uint256(_paymentCycle).ratioOf(daysInYear, 10),
        10
    );
    function percent(uint256 self, uint256 percentage, uint256 decimals)
        internal
        pure
        returns (uint256)
    {
        return (self * percentage) / percentFactor(decimals);
    }

As you can see, the decimal of ERC20 tokens is greater than 6, DOS is occured in the percent() function due to overflow.

2.The deployer can also set these two variables arbitrarily, so he can set an unfair interest rate.

Impact

The owner of the LenderGroup_Smart contract can maliciously deploy a non-lending pool or set an unfair interest rate.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L158-L213

Tool used

Manual Review

Recommendation

Add the following line to the initialize() function.

    require(interestRateUpperBound <= maxRate, "interest rate is too big.");
nevillehuang commented 3 months ago

Invalid, users can simply choose not to borrow from this market, and market owners creating market won't get any attraction to borrow due to the high interest rates. There is no reason for them to do so.

sherlock-admin3 commented 3 months ago

Escalate The protocol can only create one loan pool for each token pair of Uniswap, and because of this, the protocol cannot create a loan pool for the corresponding asset. In the worst case, the core functions of the protocol are destroyed. Therefore, this issue is considered valid.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.

FastTiger777 commented 3 months ago

Escalate The protocol can only create one loan pool for each token pair of Uniswap, and because of this, the protocol cannot create a loan pool for the corresponding asset. In the worst case, the core functions of the protocol are destroyed. Therefore, this issue is considered valid.

sherlock-admin3 commented 3 months ago

Escalate The protocol can only create one loan pool for each token pair of Uniswap, and because of this, the protocol cannot create a loan pool for the corresponding asset. In the worst case, the core functions of the protocol are destroyed. Therefore, this issue is considered valid.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.