Badger Citadel contest - m9800's results

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L830 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L13 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L132 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/interfaces/citadel/IVesting.sol#L5

Vulnerability details

Impact

Users will not be able to withdraw their funds .

Proof of Concept

When a user wants to withdraw his tokens from StakedCitadel.sol, vesting is supposed to be set and tokens are sent to the vesting contract where they are vested linearly for 21 days.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L830

This is done by calling the setupvesting() function of the imported interface IVesting.sol. The Vesting contract is StakedCitadelVester.sol and that contract doesn’t have a function called setupvesting(). The name of the function to setup a vest is vest(). So the interface doesn’t match with the contract and Users will not be able to withdraw their funds.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L13

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L132

Tools Used

Recommended Mitigation Steps

change the name of one of the functions to match with the other.

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L163 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L177 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L215 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L178

Vulnerability details

Impact

If a user doesn't set a minimum (set to 0) of citadel expected in function deposit(), can receive 0 tokens in exchange, no matter the amount of asset he's depositing.

Proof of Concept

The funding contract is used to trade citadel token with another asset. A user wanting to trade uses the deposit() function to get tokens.Deposit() uses a second function to calculate the amount Out of tokens, getAmountOut().

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L163

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L177

If funding.discount is equal to 0, the getAmountOut() function will return 0 because it will not enter in the if branch and citadelAmount_ is not declared before and its value will be 0, so 0/anything =0.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L215

So the User could receive 0 tokens in exchange of any amount of asset in.

If the user sets a non zero minimum amount in deposit(), the function will revert and users are not going to be able to buy tokens. https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L178

Tools Used

Recommended Mitigation Steps

If funding.discount is equal to 0 return citadelAmount_ = citadelAmountWithoutDiscount/ assetDecimalsNormalizationValue https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L215

Low findings :

[N01] The if branch of claim() function can ve avoided by adding a require(amount!=0, "0 amount") and it would be a better practice to revert and let the user know the error instead of doing nothing and mark the operation as succeeded

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L85

non critical:

[N01] the if branch can be avoided to simplify the code, and if in general feeBps !=0, it will save gas. https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L849