code-423n4 / 2022-04-badger-citadel-findings

0 stars 1 forks source link

QA Report #231

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1.Use assignment operator (- =) for better readability :

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

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

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

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

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

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

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

2.mintAndDistribute() will revert making the function useless when pool's value is zero

After the lockingAmount is calculated, the citadelMinter calls the stakedCitadel

deposit ():

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

inorder for shares to be distributed back to the citadelMinter contract but it will revert when and or if the value of the pool is equal to zero:

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

Check whether the pool has a non-zero value before calculating the shares.

for eg, if(pool≠0){

shares=(amount*totalSupply()) /pool;

}

3.sweep() will revert on underflow if amountLeftToBeClaimed > amount

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

Ensure to check that the aforementioned is ≤ amount in the pool. That's in the case where no extra tokens were sent but it happens that the pool's balance might not be enough to cover the amount that's left to claim.