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);
}
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); }