search

code-423n4 / 2021-04-basedloans-findings

0 stars 1 forks source link

Reward rates can be changed through flash borrows #33

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

@cmichelio

Vulnerability details

Vulnerability Details

The rewards per market are proportional to their totalBorrows which can be changed by a large holder who deposits lots of collateral, takes out a huge borrow in the market, updates the rewards, and then unwinds the position. They'll only pay gas fees as the borrow / repay can happen in the same block.

The Comptroller.refreshCompSpeeds function only checks that the single transaction is called from an EOA, but miners (or anyone if a miner offers services like flash bundles for flashbots) can still run flash-loan-like attacks by first sending a borrow tx increasing the totalBorrows, then the refreshCompSpeeds transaction, and then the repay of the borrow, as miners have full control over the transaction order of the block. The new rate will then persist until the next call to refreshCompSpeeds.

Impact

Attackers have an incentive to drive up the rewards in markets they are a large supplier/borrower in. The increased rewards that the attacker receives are essentially stolen from other legitimate users.

Recommended Mitigation Steps

Make it an admin-only function or use a time-weighted total borrow system similar to Uniswap's price oracles.

ghoul-sol commented 3 years ago

Restricting Comptroller.refreshCompSpeeds function to admin only would centralize an ability to update speeds. I think better solution is a bot that keeps track of markets utilizations and updates speeds when needed. That will also give a way to community to participate.

Also, higher rewards would mean that all participants are getting them and that would bring even more liquidity to the given market and decrease attackers earnings. Attacker could keep moving the liquidity from market to market but everyone would follow quite quickly. If that actually happens, admin has a way to stop the rewards and make refreshCompSpeeds admin-only function as last resolution because comptroller is using proxy pattern.