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

0 stars 1 forks source link

In the citadelMinter there is a missing check on the `mintAndDistribute ()` function #190

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

in (1) there is no verification that fundingBps,stakingBps,lockingBps was set, and therefore the distribution could be lost for everyone. We should make sure that the functionsetCitadelDistributionSplit was called

Proof of Concept

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

Tools Used

hardhat test

Recommended Mitigation Steps

Add a require that _fundingBps + _stakingBps + _lockingBps == MAX_BPS

GalloDaSballo commented 2 years ago

Why would the 3 bps need to be 100%?

@dapp-whisperer wdyt?

shuklaayush commented 2 years ago

I guess we can add that check. Would characterize this as low though

jack-the-pug commented 2 years ago

If fundingBps, stakingBps, lockingBps are not set, then it means that they are 0, right? So, it's not a bug that the distribution will mint 0.

I'll downgrade this to QA.