hats-finance / StakeWise-0xd91cd6ed6c9a112fdc112b1a3c66e47697f522cd

Liquid staking protocol for Ethereum
Other
0 stars 0 forks source link

User can utilize a flash loan to manipulate treasury fee #114

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @evokid Submission hash (on-chain): 0x9fea796990c8d463bc986fb64e22a52ed0500f20c3731e7bc97921043f499521 Severity: high

Description: Class High

Description

cumulativeFeePerShare of _osToken is responsible to add the treasury fee to the position in the protocol. However, there is a specific scenario that can attacker use to avoid paying the right treasury fee according to his osETH holding time.

as we realize in burnOsToken() the treasury fee will be calculated for the whole time that user hold until burning the osETH token, but this could be manipulated by the scenario below and pay less treasury fee to the protocol.

Attack Scenario

  1. Sam deposited 100 ETH in the vault token

  2. He minted 90 osETH tokens since he can get 90% of the staked ETH in osETH.

  3. One year passed 360 days with holding osETH tokens.

  4. Now Sam wants to manipulate treasury fee that he should pay when burning his osETH .

  5. Sam create a contract and include next actions in one transaction:

  6. He gets a Flash loan of 1k ETH

  7. Sam deposited 1k ETH in the vault

  8. He minted 900 osETH as well

  9. What done in the protocol now that _totalShares of osETH token increased in huge value.

  10. Sam burns his 90 osETH (point 2)

  11. Sam paid the treasury fee for 90 ETH which deceased far from the right value to pay for 360 days because _totalShares is too high.

  12. Now Sam burn 900 osETH.

  13. Sam redeem from the Vault to pay back the flash loan.

Because _totalShares had a big increase, the protocol will calculate less treasury fees of 360 days period to the attacker to pay which is considered as an abuse to the protocol fees.

Attachments

  1. Proof of Concept (PoC) cumulativeFeePerShare() which calls _unclaimedAssets() L336
function _unclaimedAssets() internal view returns (uint256) {
    // calculate time passed since the last update
    uint256 timeElapsed;
    unchecked {
      // cannot realistically underflow
      timeElapsed = block.timestamp - _lastUpdateTimestamp;
    }
    if (timeElapsed == 0) return 0;
    return Math.mulDiv(avgRewardPerSecond * _totalAssets, timeElapsed, _wad);
  }
}

must calculate the correct profitAccrued to calculate treasury shares later in cumulativeFeePerShare() L216-L245

    // add treasury fee to the position
    position.shares = SafeCast.toUint128(
      Math.mulDiv(position.shares, cumulativeFeePerShare, position.cumulativeFeePerShare)
    );

but the attacker decreased what he must pay by the attack scenario mentioned above, which become less far from the expected or correct value.

Recommendations:

tsudmi commented 1 year ago

The fee for a year will be calculated correctly during "He minted 900 osETH as well". Before Sam mints 900 osETH, the protocol will update his yearly fee

evokid commented 1 year ago

Hey @tsudmi, not sure what do you mean by that. would you please explain it in better example? I would like it to be in detail not just general describing. the issue is about reducing the fees for 90 osETH not 900, the flash loan is to assist what already minted before to avoid the fees for it (point .2)