Platform: Code4rena
Start Date: 06/01/2022
Pot Size: $60,000 USDC
Total HM: 20
Participants: 33
Period: 7 days
Judge: LSDan
Total Solo HM: 9
Id: 67
League: ETH
Rank: 13/33
Findings: 6
Award: $1,561.50
🌟 Selected for report: 1
🚀 Solo Findings: 0
danb
https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/strategy/NonUSTStrategy.sol#L78
the function updateInvested is vulnerable to sandwich attacks, becuase the min return param in curvePool.exchange_underlying is set to zero.
a large amount of the investment can be stolen.
check the deviation of the spot price from the twap, if it's too high, revert.
#0 - naps62
2022-01-11T18:45:56Z
duplicate of #7
90.0579 USDC - $90.06
danb
reentrancy is possible in _createClaim in
https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/Vault.sol#L484
it is possible to reenter _createDeposit before _depositGroupIds is incremented and cause many deposits with the same id. thus confusing the client of the event.
add nonReentrant modifier.
#0 - r2moon
2022-01-07T16:34:35Z
105.9124 USDC - $105.91
danb
totalUnderlying() includes the invested assets, they are not in the contract balance.
when a user calls withdraw, claimYield or unsponsor, the system might not have enough assets in the balance and the transfer would fail.
especially, force unsponsor will always fail, because it tries to transfer the entire totalUnderlying(), which the system doesn't have:
https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/Vault.sol#L391
when the system doesn't have enough balance to make the transfer, withdraw from the strategy.
#0 - gabrielpoca
2022-01-12T10:17:47Z
I'm not sure this is an issue. We are aware of it, and redeeming from the strategy won't fix it because it is asynchronous. This is why we have an investment percentage.
#1 - dmvt
2022-01-27T13:12:45Z
This one is a hard issue to size, but I'm going to go with the medium risk rating provided by other wardens reporting this issue. This seems to amount to a bank run like issue similar to what can happen with DeFi lending protocols.
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
If the invested assets are compromised or locked, this could result in a loss of funds. Users of the protocol should be made aware of the risk. This risk exists with many DeFi protocols and probably shouldn't be a surprise to most users.
🌟 Selected for report: kenzo
Also found by: danb, harleythedog
danb
https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/Vault.sol#L114
strategy change requires the invested assets to be zero. anyone can make it fail by transferring aust to the strategy.
#0 - naps62
2022-01-13T19:32:29Z
duplicate of #91
danb
investedAssets() doesn't substract the fees owed to the treasury, this makes the system think that it has more than it really has.
consider the following scenario:
perfFeePct is 20%.
the system generated 1M dollars yield in aust that it didn't redeem yet.
besides that, there are 1M dollars in the system, so totalUnderlyingMinusSponsored() is 2M.
after the 1M yield aust are redeemed, 20% will be taken so totalUnderlyingMinusSponsored() will be 1.8M
people who invested before the redeem will not be able to withdraw regularly because their shares are worth less than their deposit amount and they can only withdraw with force, therefore losing 10% of their investment. because 1.8M = 90% * 2M.
#0 - r2moon
2022-01-11T16:09:13Z