search

code-423n4 / 2021-08-notional-findings

3 stars 0 forks source link

Time window must be chosen carefully #19

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

tensors

Vulnerability details

Impact

The variable timeWindow in the formula of valuation/README.md must be chosen carefully. In fact, it should probably vary with the amount of user volume the protocol has on each asset.

Consider what happens when timeWindow is very small (~5min). A price manipulation attack can skew the price on the lastImpliedRate, and if the price is sufficiently altered the attacker will only need to wait a few blocks (say 1 min) until the formula gives sufficient weight to his skewed price, allowing the attacker to profit.

Alternatively, if the timeWindow is too large and the last trade was a while ago (low volume), the lastImpliedRate will be different from the oracle rate, allowing an attacker to profit since the formula gives all the weight to the lastImpliedRate.

If their is sufficient user volume both of the above cases become less of an issue.

Proof of Concept

Based on the formula here: https://github.com/code-423n4/2021-08-notional/blob/main/contracts/internal/valuation/_README.md

Recommendations

Allow the owner the adjust the time window. Carefully consider the length of the window and monitor if the protocol is losing assets due to this.

T-Woodward commented 3 years ago

The time window is a governance parameter and can be updated by Notional governors as market conditions change. The time window must be chosen carefully just like any other risk parameter, but this isn’t a bug. This is just the reality of a protocol that requires its governors to engage in risk management. This issue is equivalent to saying that the collateral haircut must be chosen carefully or else the protocol could become insolvent. This is true of course, but it’s not a bug.

ghoul-sol commented 3 years ago

I don't see a bug or exploit here. Making this invalid.