Sandclock contest - camden's results

Handle

camden

Vulnerability details

Impact

The impact of this is that users can get significantly more UST withdrawn than they would be alotted if they had done non-reentrant withdraw calls.

Proof of Concept

Here's an outline of the attack:

Assume the vault has 100 UST in it. The attacker makes two deposits of 100UST and waits for them to be withdrawable. The attacker triggers a withdraw one of their deposit positions. The vault code executes until it reaches this point: https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L565 Since the attacker is the claimer, the vault will call back to the attacker. Inside onDepositBurned, trigger another 100 UST deposit. Since claimers.onWithdraw has already been called, reducing the amount of shares, but the UST hasn't been transferred yet, the vault will compute the amount of UST to be withdrawn based on an unexpected value for _totalUnderlyingMinusSponsored (300). https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L618

After the attack, the attacker will have significantly more than if they had withdrawn without reentrancy.

Here's my proof of concept showing a very similar exploit with deposit, but I think it's enough to illustrate the point. I have a forge repo if you want to see it, just ping me on discord. https://gist.github.com/CamdenClark/abc67bc1b387c15600549f6dfd5cb27a

Tools Used

Forge

Recommended Mitigation Steps

Reentrancy guards.

Also, consider simplifying some of the shares logic.

Handle

camden

Vulnerability details

Impact

In short, the NonUSTStrategy is vulnerable to attacks by flash loans on curve pools.

Here's an outline of the attack:

• Assume there is a vault with DAI underlying and a NonUSTStrategy with a DAI / UST curve pool
• Take out a flash loan of DAI
• Exchange a ton of DAI for UST
• The exchange rate from DAI to UST has gone up (!!)
• Withdraw or deposit from vault with more favorable terms than market
• Transfer back UST to DAI
• Repay flash loan

Proof of Concept

Here is my proof of concept: https://gist.github.com/CamdenClark/932d5fbeecb963d0917cb1321f754132

I can provide a full forge repo. Just ping me on discord.

Exploiting this line: https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/strategy/NonUSTStrategy.sol#L135

Tools Used

Forge

Recommended Mitigation Steps

Use an oracle

Handle

camden

Vulnerability details

Impact

The impact of this is that users can get significantly more shares than they would be alotted if they had done non-reentrant deposit calls.

Proof of Concept

Here's an outline of the attack:

Assume the vault has 100 UST in it. The attacker triggers a deposit of 100UST. The vault code executes until it reaches this point: https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L484 Since the attacker is marked as a beneficiary, the vault will call back to the attacker. Inside onDepositMinted, trigger another 100 UST deposit. Since the claim shares have already been minted, but the UST hasn't been transferred yet, the vault will give out 200 shares for this deposit, as opposed to 100, because _totalUnderlyingMinusSponsored still has the unexpected value of 100. https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L618

After the attack, the attacker will have 300 of 400 total shares, but only have contributed 200 of 300 UST principal.

Here's my proof of concept. I have a forge repo if you want to see it, just ping me on discord. https://gist.github.com/CamdenClark/abc67bc1b387c15600549f6dfd5cb27a

Tools Used

Forge

Recommended Mitigation Steps

Reentrancy guards.

Also, consider simplifying some of the shares logic.

Handle

camden

Vulnerability details

Impact

The impact of this is that a user could lock sponsorship money permanently and be unable to unlock it. No other users' funds are at risk.

Proof of Concept

Here's an outline of the attack:

The attacker makes a sponsor deposit. The attacker gets an ERC721 safe transfer call. The attacker uses this to reenter with an unsponsor on the token id used with the sponsor deposit. The token minted for the sponsor deposit is burned by unsponsor. The UST is transferred anyways.

Here's my proof of concept showing a very similar exploit with deposit, but I think it's enough to illustrate the point. The main difference is that you would have to use the onERC721Received call to perform reentrancy. I have a forge repo if you want to see it, just ping me on discord. https://gist.github.com/CamdenClark/abc67bc1b387c15600549f6dfd5cb27a

Tools Used

Forge

Recommended Mitigation Steps

Reentrancy guards.

Handle

camden

Vulnerability details

Impact

Ideally use _safeMint everywhere and make sure you're safe against reentrancy.

Proof of Concept

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/vault/Claimers.sol#L63 https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/vault/Depositors.sol#L53