Inverse Finance contest - RaoulSchaffranek's results

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L287

Vulnerability details

Impact

While a user borrows DOLA, his debt position in the DBR contract accrues more debt over time. However, Solidity contracts cannot update their storage automatically over time; state updates must always be triggered by externally owned accounts. For this reason, the DBR contract cannot accurately represent a user's debt position in its storage at all times. Instead, the contract offers a method accrueDueTokens that, when called, updates the internal storage with the debts that accrued since the last update. This method is called before all critical financial operations that depend on an accurate value of the accumulated deficit in the contract's storage. On top, this method can also be invoked permissionless at any time. Suppose a borrower manages to call this function periodically and keep the time difference between updates short. In that case, a rounding error in the computation of the accrued debt can cause the expression to round down to zero. In this case, the user successfully avoided paying interest on his debt.

Proof of Concept

For reference, here is the affected code:

    function accrueDueTokens(address user) public {
        uint debt = debts[user];
        if(lastUpdated[user] == block.timestamp) return;
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
        dueTokensAccrued[user] += accrued;
        totalDueTokensAccrued += accrued;
        lastUpdated[user] = block.timestamp;
        emit Transfer(user, address(0), accrued);
    }

The problem is that the function updates the lastUpdated[user] storage variable even when accrued is 0.

Example

Let's assume that the last update occurred at t_0. Further assume that the next update occurs at t_1 with t_1 - t_0 = 12s. (12s is the current Ethereum block time) Suppose that the user's recorded debt position at t_0 is 1,000,000 wei`. Then the accrued debt formula gives us the following:

accrued = (t_1 - t_0) * debt / 365 days
        = 12          * 1,000,000 / 31,536,000
        = 1,000,000 / 31,536,000
        = 0 (because unsigned integer division rounds down)

Maximizing profit

The accrued debt formula rounds towards zero if we have (t_1 - t_0) * debt < 365 days. This gives us a method to compute the maximal debt that we can deposit to make the attack more efficient:

debt_max = 365 days / 12s -1 = 2,627,999

Notice that an attacker is not limited to these small loans. He can split a massive loan into multiple small loans, capped at 2,627,999. To borrow X tokens (where X is given in WEI), we can compute the number of needed loans as:

#loans = X / 2,627,999

For example, to borrow 1 DOLA:

#loans = 10^18 / 2,627,999 = 380517648599

To borrow 1,000,000 DOLA we would thus need 380,517,648,599,000,000 small loans.

Economical feasibility

The attack would be economically feasible if the costs of the attack were lower than the interest that accrued throughout the successful attack. The dominating factor of the attack costs is the gas costs which the attacker needs to pay to update the accrued interest of the small loans every second. A clever attacker would batch as many updates into a single transaction as possible to minimize the gas overhead of the transaction. Still, at the current block time (12s), gas price (7 gwei), block gas limit (30,000,000), and current ETH price ($1,550.80), it's hardly imaginable that this attack is economically feasible at the moment.

Risk parameters

However, all these values could change in the future. And if we look at other networks, Layer2 or EVM compatible Layer1, the parameters might be different today.

Also, notice that if the contract were used to borrow a different asset than DOLA, the numbers would look drastically different. The risk increases with the asset's price and becomes bigger the fewer decimals the token uses. For example, to borrow 1 WBTC (8 decimals), we would only need 39 small loans:

#loans = 10^8 / 2,627,999 ~39

And to borrow WBTC worth $1,000,000 at a price of 20,746$/BTC, we would need 1864 small loans.

#loans ~= 49*10^8 / 2,627,999 ~= 1864

Foundry

The following test demonstrates how to avoid paying interest on a loan for 1h. A failing test means that the attack was successful.

$ git diff src/test/DBR.t.sol
diff --git a/src/test/DBR.t.sol b/src/test/DBR.t.sol
index 3988cf7..8779da7 100644
--- a/src/test/DBR.t.sol
+++ b/src/test/DBR.t.sol
@@ -25,6 +25,20 @@ contract DBRTest is FiRMTest {
         vm.stopPrank();
     }
 
+    function testFail_free_borrow() public {
+        uint borrowAmount =  2_627_999;
+
+        vm.prank(address(market));
+        dbr.onBorrow(user, borrowAmount);
+
+        for (uint i = 12; i <= 3600; i += 12) {
+            vm.warp(block.timestamp + 12);
+            dbr.accrueDueTokens(user);
+        }
+        assertEq(dbr.deficitOf(user), 0);
+    }
+
+
     function testOnBorrow_Reverts_When_AccrueDueTokensBringsUserDbrBelow0() public {
         gibWeth(user, wethTestAmount);
         gibDBR(user, wethTestAmount);

Output:

$ forge test --match-test testFail_free_borrow -vv
[⠆] Compiling...
[⠊] Compiling 1 files with 0.8.17
[⠢] Solc 0.8.17 finished in 2.62s
Compiler run successful

Running 1 test for src/test/DBR.t.sol:DBRTest
[FAIL. Reason: Assertion failed.] testFail_free_borrow() (gas: 1621543)
Test result: FAILED. 0 passed; 1 failed; finished in 8.03ms

Failing tests:
Encountered 1 failing test in src/test/DBR.t.sol:DBRTest
[FAIL. Reason: Assertion failed.] testFail_free_borrow() (gas: 1621543)

Encountered a total of 1 failing tests, 0 tests succeeded

Classified as a high medium because the yields can get stolen/denied. It's not high risk because I don't see an economically feasible exploit.

Tools Used

VSCode, Wolramapha, Foundry

Recommended Mitigation Steps

• Document the risks transparently and prominently.
• Re-evaluate the risks according to the specific network parameters of every network you want to deploy to.
• Do not update the lastUpdated timestamp of the user if the computed accrued amount was zero.

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L61 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L53

Vulnerability details

Governance can drain all escrows

Impact

The governance account controlling the oracle can change the underlying price feed at any time. This capability can be accidentally misused or intentionally abused to harm the protocol in different ways, including the possibility of permanently halting the protocol and draining all assets from escrows. The same security risks also apply to attackers who gain control over a Chainlink feed in a way that allows them to report false price information or causes price lookups to revert.

Classified as a medium risk because assets can be stolen and temporarily or permanently locked into escrows, but the attack can only be executed by privileged accounts.

Proof of Concept

Example 1: Halting the protocol

Certain functions depend on price information from the oracle. If the respective lookup functions fail, the entire transaction will revert. In particular, this makes the following market functions unavailable: withdrawing, borrowing, replenishing, and liquidations. This can, for example, happen if the price feed address points to the zero address. Notice, that this can also occur accidentally by an honestly operating governance account because there is no input validation when setting the price feed address.

It should be noted that the governance account has permission to call the pause functions on markets. When paused, the market should make it impossible to borrow, but withdrawing, replenishing, and liquidations should still be possible. By abusing the privilege to change the chainlink feed, the governance account can circumvent this restriction and essentially pause all market operations - the only exception being deposits.

Example 2: Draining all assets from escrows

The governance account has complete control over the prices used within the system. It can either set fixed prices for collateral assets which will take priority over price oracles, or it could install a malicious price oracle. Consequently, the governance can trigger undue liquidations by increasing the price and stealing the collateral from the lenders. It could also decrease the cost of borrowing at a cheaper rate.

Tools Used

VSCode

Recommended Mitigation Steps

• Perform a sanity check when changing the oracle. For example, try calling latestRoundData and decimals and check if the return values are coherent. Notice that does not prevent an attacker from switching intentionally to a malicious oracle. Still, it can avoid certain cases where an honestly operating governance account accidentally sets the chainlink feed to the wrong address.

• Consider adding a timelock to critical protocol parameters, such as fixed prices and Chainlink feeds in the protocol. For example, when the governance wants to change the oracle feed, the protocol could wait for X hours before the effects are put into action. This allows users to respond to the proposed change and take immediate action before the new price feed is activated. Document the behavior transparently and prominently, and publicly announce intended changes to critical protocol parameters early.

• Also, monitor the Chainlink feeds for malicious behavior.

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L87 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L121

Vulnerability details

Impact

The Oracle contract always tries to normalize prices to DOLA. More precisely, the oracle has to account for different decimals in the price feed, the asset token, and the DOLA token. It uses the following formula:

uint8 decimals = 36 - feedDecimals - tokenDecimals;
uint normalizedPrice = price * (10 ** decimals)

The problem is that the formula underflows when feedDecimals + tokenDecimals > 36. In this case, the viewPrice and getPrice functions will revert.

One critical consequence is that withdrawals, liquidations, replenishment, and borrows on markets no longer work. In particular, collateral inside escrows cannot be unlocked.

Notice that the governance can recover from this situation by switching to a different price feed or setting a fixed price for the underlying collateral.

Classified as a medium risk because assets are not directly at risk, but the availability of the protocol is affected.

Proof of Concept

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L87 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L121

Tools Used

VSCode

Recommended Mitigation Steps

• Check compatibility with the Chainlink price feed before setting the oracle address.
• Use a different formula to account for the differences in decimals that does not underflow/overflow.