Badger Citadel contest - remora's results

Lines of code

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

Vulnerability details

Impact

Function getAmountOut(_assetAmountIn) fails to handle the case for funding.discount == 0

Proof of Concept

 function getAmountOut(uint256 _assetAmountIn)
        public
        view
        returns (uint256 citadelAmount_)
    {
        uint256 citadelAmountWithoutDiscount = _assetAmountIn * citadelPriceInAsset;

        if (funding.discount > 0) {
            citadelAmount_ =
                (citadelAmountWithoutDiscount * MAX_BPS) /
                (MAX_BPS - funding.discount);
        }
        
        // @audit-issue 
        // if `funding.discount == 0`, `citadelAmount_` will be returned with default initialized value (i.e 0)

        citadelAmount_ = citadelAmount_ / assetDecimalsNormalizationValue;
    }

Tools Used

Manual review

Recommended Mitigation Steps

1. Comment is misleading for @param _minCitadelOut

• In line https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L160 the comment for _minCitadelOut describe it as ID of DAO.

2. TREASURY_GOVERNANCE_ROLE is not used anywhere in the contract KnightingRound.sol and can be omitted.

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