Badger Citadel contest - pedroais'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/interfaces/citadel/IVesting.sol#L5 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L132

Vulnerability details

Impact

Users can't withdraw their funds

Proof of Concept

When withdrawing in staking the contract will send funds to vesting using setupVesting(). This is defined in the vesting interface. On the other hand, the vesting implementation calls this function vest() so setupVesting() isn't implemented.

All withdraws will revert. This doesn't come up in tests since the staking wasn't tested (at least in provided tests).

Recommended Mitigation Steps

Give the function the same name in interface and implementation

Lines of code

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

Vulnerability details

Impact

Users are unable to buy citadel in funding if the discount is 0.

Proof of Concept

In the funding contract if the discount is 0 then getAmountOut will always return 0 and users won't be able to use funding to buy citadel.

If discount is 0 then the if is never true and citadelAmount_ is never set. Then citadelAmount_ = citadelAmount_ / assetDecimalsNormalizationValue will always be 0. Since there is a minAmountOut input the buy will revert so no tokens will be lost but buying won't be possible.

In talks with the sponsor, they confirmed this is unintended and the discount should be settable to 0 without stopping funding.

I consider this a medium issue since assets aren't at direct risk but the function of the protocol is impacted.

In addition, funds can be at risk if a user buys with minAmountOut = 0 they will lose all funds and receive no tokens in return.

Recommended Mitigation Steps

Change to : function getAmountOut(uint256 assetAmountIn)///@audit-ok public view returns (uint256 citadelAmount) { uint256 citadelAmount_ = assetAmountIn * citadelPriceInAsset; if (funding.discount > 0) { citadelAmount = (citadelAmount_ * MAX_BPS) / (MAX_BPS - funding.discount); }

    citadelAmount_ = citadelAmount_ / assetDecimalsNormalizationValue;
}