search

code-423n4 / 2022-01-sandclock-findings

0 stars 0 forks source link

Incorrect share accounting #192

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

gzeon

Vulnerability details

Impact

Unlike Yearn-like vault that collect fee upon harvest, performance fee on SandClock strategy is claimed only on aUST redeem. Assuming a non-decreasing vault TVL, this fee will rarely be claimed and will inflate the actual vault TVL. The inflated TVL will lead to later depositor received less share than they should due to some share is fee-payable immediately.

Proof of Concept

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L122 https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/strategy/BaseStrategy.sol#L195

naps62 commented 2 years ago

While I agree with the issue, this doesn't seem at all "high risk". The real issue is that TVL shown to the user is actually slightly off, since it's not accounting for the eventual fee that will end up being taken. This is different from loss or lock of funds, in my opinion.

dmvt commented 2 years ago

I agree with the sponsor on this. This is low risk. No assets are at risk but the user may misunderstand what will happen.

1 — Low: Low: Assets are not at risk. State handling, function incorrect as to spec, issues with comments.
r2moon commented 2 years ago

This issue has been fixed.