Badger Citadel contest - 0xBug's results

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

After depositing into StakedCitadel user would be unable to withdraw their funds. Even though the contract is upgradeable this is a High Risk issue as a error in _withdraw() would cause the funds to be locked into the contract causing the team to go through chaos of timelock / multisig to upgrade the contract + cause reputation damage.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

In function https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L808 when setting up the user vesting period https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L830 there is no function as setupVesting in https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L132

Recommended Mitigation Steps

• change function name in StakedCitadelVester::vest() -> StakedCitadelVester::setupVesting()

OR

• Edit IVesting.sol::setupVesting to

function vest(
        address recipient,
        uint256 _amount,
        uint256 _unlockBegin
    ) external;

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. whenever there is no discount on funding, user facing function ie. deposit(), getAmountOut() breaks in Funding.sol

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

When funding.discount is zero citadelAmount_ is zero therefore the out amount would be always zero

Recommended Mitigation Steps

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

 if (funding.discount > 0) {
            citadelAmount_ =
                (citadelAmountWithoutDiscount * MAX_BPS) /
                (MAX_BPS - funding.discount);
} else {
    citadelAmount_ = citadelAmountWithoutDiscount;
}

• Cache funding.discount in Funding.sol::getAmount(). Would save ~100 gas. Change -

    function getAmountOut(uint256 _assetAmountIn)
        public
        view
        returns (uint256 citadelAmount_)
    {
        uint256 citadelAmountWithoutDiscount = _assetAmountIn * citadelPriceInAsset;
        uint256 discount = funding.discount;
        if (discount != 0) {
            citadelAmount_ =
                (citadelAmountWithoutDiscount * MAX_BPS) /
                (MAX_BPS - discount);
        } else {
            citadelAmount_ = citadelAmountWithoutDiscount;
        }

        citadelAmount_ = citadelAmount_ / assetDecimalsNormalizationValue;
    }

• Do not initialize to zero https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/CitadelMinter.sol#L180 Change -

uint256 lockingAmount;
uint256 stakingAmount;
uint256 fundingAmount;