code-423n4 / 2021-07-spartan-findings

0 stars 0 forks source link

DaoVault/ BondVault/ SynthVault are vulnerable to price manipulation. #142

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

jonah1005

Vulnerability details

Impact

DaoVault/ BondVault/ SynthVault calculates weight based on AMM spot price that's vulnerable to flash loan attacks. Though three contracts pose different threat levels to users' funds, the root cause and the fix of the three contracts are the same.

BondVault:

Dao's function bond(address asset, uint256 amount) would calculate the input token value and deposit the same value of Sparta into the pools. This would force the Dao contract to invest in the pool at a bad price and creates arbitrage space for attackers.

We assume there's a Pool start with Sparta: Dai = 100:100, and Alice is the only lpholder.

  1. Alice buy dai with Sparta and make Sparta: Dai = 10000: 1
  2. Alice calls bond and deposit 10 dai.
  3. Dao pools would deposit 10 dai and 100000 Sparta to the pool; The pool currently has 110000 Sparta and 11 dai.
  4. Alice could by back to Sparta with 99 dai she gets at step 1 and gets 99000.

We assume a simple x*y=k with no transaction fee to simplify the issue. Though it's harder for hackers to exploit Sparta because a different curve is applied, it's still profitable.

Severity: High

SynthVault

The SynthVault contract calculates users' weight by calling calcSpotValueInBase. An attacker can increase his weight by mutating the price through a flash loan.

DaoVault

The DaoVault contract calculates users' weight by calling getPoolShareWeight which users AMM spot price. An attacker can increase his weight by mutating the price through flash loan.

Luckily, attackers would be hard to drain the reward fromDaoVault and SynthVault. Mutating AMM pool is expensive for the trading fee of a big trade is really high. However, if a Pool somehow only has one lp holder, it would be profitable. An attacker can mutate the pool to an extreme price and pays no trading fee.

I consider the severity of SynthVault and DaoVault is medium.

Proof of Concept

Dao calculates price at: https://github.com/code-423n4/2021-07-spartan/blob/main/contracts/Dao.sol#L239-L254 Dao invests at a bad price at https://github.com/code-423n4/2021-07-spartan/blob/main/contracts/Dao.sol#L257-L273

SynthVault calculates price at: https://github.com/code-423n4/2021-07-spartan/blob/main/contracts/synthVault.sol#L102-L116

BondVault calculates price at: https://github.com/code-423n4/2021-07-spartan/blob/main/contracts/BondVault.sol#L77-L88

Tools Used

Hardhat

Recommended Mitigation Steps

It seems that the current protocol doesn't implement a safe oracle contract. I recommend implementing a TWAP price oracle and use the oracle price instead of AMM spot price.

verifyfirst commented 3 years ago

When a user bonds, it uses swap rate - not spot rate. This invalidates the flashloan attack theory described above. For synth and dao vault weights, they use spot value as by design because the weight is only used for accounting purposes and doesn't need to be susceptible to smoothing.

ghoul-sol commented 3 years ago

per sponsor comment, invalid