Vader Protocol contest - danb's results

Handle

danb

Vulnerability details

https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L165

nativeAsset.safeTransferFrom(from, address(this), nativeDeposit);

mintSynth has a from parameter, this is where they take the money for the transaction. If an address has allowance for the contract, anyone can use it and take it using mintSynth.

Impact

If a user sets more allowance than they have to, for example if they want to call mintSynth many times and don't want to call approve again, then their funds can be stolen by anyone. also, if a user wants to call mintSynth, and call approve in another transaction (so they don't have to create a contract), then mintSynth can be frontrun to steal their funds.

Proof of Concept

consider the following scenario: A user wants to call mintSynth. They know that they must approve nativeAsset to the contract, so they execute an approve transaction, so that later they will execute mintSynth. when they submit the approve transaction, a malicious actor spots this, and sends a mintSynth transaction where from is equal to the user address. the hacker was able to steal the user's funds!

Tools Used

manual review

Recommended Mitigation Steps

change to:

nativeAsset.safeTransferFrom(msg.sender, address(this), nativeDeposit);

Handle

danb

Vulnerability details

https://github.com/code-423n4/2021-12-vader/blob/main/contracts/lbt/LiquidityBasedTWAP.sol#L131 for tokens that are not updating in syncVaderPrice because their updatePeriod is greated than timeElapsed, their liquidty weight will be zero, it will make the vader price wrong when calling getVaderPrice and it can be exploited.

Recommended Mitigation Steps

move this line: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/lbt/LiquidityBasedTWAP.sol#L140 before this line: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/lbt/LiquidityBasedTWAP.sol#L131

Handle

danb

Vulnerability details

https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L334 on the first deposit, the total liquidity is set to nativeDeposit. this might be a very low number compared to foreignDeposit. It can cause a denial of service of the pair.

Impact

A pair can enter a denial of service state.

Proof of Concept

consider the following scenario: the owner of the pool calls setFungibleTokenSupport for a new token, for example weth. a malicious actor calls mintFungible, with nativeDeposit == 1 and foreignDeposit == 10 ether. totalLiquidityUnits will be 1. the pool can be arbitraged, even by the attacker, but totalLiquidityUnits will still be 1. this means that 1 liquidity token is equal to all of the pool reserves, which is a lot of money. It will cause a very high rounding error for anyone trying to mint liquidity. then, anyone who will try to mint liquidity will either:

1. fail, because they can't mint 0 liquidity if their amount is too small.
2. get less liquidity tokens than they should, because there is a very high rounding error, and its against new liquidity providers. The rounding error losses will increase the pool reserves, which will increase value of liquidity tokens, so the hacker can even profit from this.

after this is realised, no one will want to provide liquidity, and since the pair cannot be removed or replaced, it will cause denial of service for that token forever.