Sandclock contest - hyh's results

The Next Generation of Wealth Creation.

General Information

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

Sandclock

Findings Distribution

Researcher Performance

Rank: 17/33

Findings: 2

Award: $680.56

🌟 Selected for report: 1

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: camden

Also found by: Ruhum, WatchPug, cccz, cmichel, danb, defsec, harleythedog, hyh, kenzo, leastwood, palina, pauliax, pmerkleplant, ye0lde

Labels

bug
duplicate
3 (High Risk)

Awards

90.0579 USDC - $90.06

External Links

Handle

hyh

Vulnerability details

Impact

Sandwich attack is possible: an attacker can track market order and perform it whenever order amount to be executed is big enough to compensate for exchange manipulation costs. On Curve it is less profitable and this way less probable due to low slippage, but the possibility exists.

Proof of Concept

No minimum return is used when Curve is called to do the UST swaps:

doHardWork -> _swapUnderlyingToUst use 0 as min accepted return:

https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/strategy/NonUSTStrategy.sol#L78-83

finishRedeemStable ->_swapUstToUnderlying also use 0 as min return:

https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/strategy/NonUSTStrategy.sol#L94

Add a slippage argument to doHardWork and finishRedeemStable functions and supply minimum accepted return to Curve instead of hard coded 0

#0 - naps62

2022-01-11T18:44:40Z

duplicate of #7

Findings Information

🌟 Selected for report: hyh

Labels

bug
1 (Low Risk)
disagree with severity
sponsor vault

Awards

590.4972 USDC - $590.50

External Links

Handle

hyh

Vulnerability details

Proof of Concept

underlying() function description repeats minLockPeriod()’s one:

https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/vault/IVault.sol#L73-75

Set it to same line as is used in Vault, ‘Underlying ERC20 token accepted by the vault':

https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/Vault.sol#L48

#0 - r2moon

2022-01-11T15:54:59Z

I agree with this issue, but it is not an risk.

#1 - dmvt

2022-01-28T20:12:32Z

No assets at risk, but it is an issue with a comment.

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

#2 - naps62

2022-02-16T15:51:45Z

already fixed

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter