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

0 stars 1 forks source link

Funding.getAmountOut returns zero when there is no discount set #219

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

User provided asset funds will be lost, i.e. 100% to be frozen in the contract, as the system will not give away any Citadel in return.

The issue is that when Funding's funding.discount is zero the getAmountOut() will return zero for any given _assetAmountIn so nothing will be deposited on the behalf of msg.sender.

Placing severity to be high as that's clear violation of system logic, while the impact is user funds become frozen in the contract and had to be recovered manually.

Proof of Concept

Whenever funding.discount is zero, the citadelAmount_ will be zero as well by default, as citadelAmount_ is defined only when funding.discount > 0:

uint256 citadelAmountWithoutDiscount = _assetAmountIn * citadelPriceInAsset;

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

citadelAmount_ = citadelAmount_ / assetDecimalsNormalizationValue;

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

This way getAmountOut() will produce nothing for any amount of asset invested:

https://github.com/code-423n4/2022-04-badger-citadel/blob/main/src/Funding.sol#L177

Notice that the issue was introduced lately with discounts, as it didn't existed in the previous version:

https://github.com/code-423n4/2022-02-badger-citadel/blob/main/contracts/TokenSaleUpgradeable.sol#L221-L223

Recommended Mitigation Steps

Consider either replacing citadelAmountWithoutDiscount with citadelAmount_ or adding else to if (funding.discount > 0) {...} else {citadelAmount_ = citadelAmountWithoutDiscount;} logic

GalloDaSballo commented 2 years ago

Has been mitigated in subsequent PR

jack-the-pug commented 2 years ago

Dup #149