code-423n4 / 2023-04-caviar-findings

9 stars 4 forks source link

There is no check for the protocolFeeRate limit in the Factory contract #958

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/Factory.sol#L139-L143

Vulnerability details

Impact

Each pool must send a protocolFeeRate amount of fee to the Factory contract. However, the value of protocolFeeRate is not checked when set in the function:

/// @notice Sets the protocol fee that is taken on each buy/sell/change. It's in basis points: 350 = 3.5%.
/// @param _protocolFeeRate The protocol fee.
function setProtocolFeeRate(uint16 _protocolFeeRate) public onlyOwner {
    protocolFeeRate = _protocolFeeRate;
}

The admin may make a mistake and set a value above 10,000, which would temporarily break all pools, as the protocol would try to charge users a fee exceeding the amount of the transaction.

Recommended Mitigation Steps

It is recommended to set a limit on the protocolFeeRate in the setProtocolFeeRate() function.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #541

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-b