code-423n4 / 2023-03-polynomial-findings

2 stars 1 forks source link

LiquidityPool.sol#L657 : setFees() could be abused to steal the funds when there is huge transacion is happening. #230

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L657-L675

Vulnerability details

Impact

Set fee functions can be set to any arbitrary value when the transaction is happening. There are more possibility that user could lose most of all of their hard earned funds.

Proof of Concept

Below functions can be called by autheraised person to fix the fee during the deposit/withdraw.

function setFees(uint256 _depositFee, uint256 _withdrawalFee) external requiresAuth {
    emit UpdateFees(depositFee, _depositFee, withdrawalFee, _withdrawalFee);
    depositFee = _depositFee;
    withdrawalFee = _withdrawalFee;
}

/// @notice Set base trading fee
/// @param _baseTradingFee New base trading fee
function setBaseTradingFee(uint256 _baseTradingFee) external requiresAuth {
    emit UpdateBaseTradingFee(baseTradingFee, _baseTradingFee);
    baseTradingFee = _baseTradingFee;
}

/// @notice Update dev fee
/// @param _devFee New dev fee
function setDevFee(uint256 _devFee) external requiresAuth {
    emit UpdateDevFee(devFee, _devFee);
    devFee = _devFee;
}

The issue here is, this opens the case where admin can abuse with excessive power.

Lets say, if a user wants to withdraw huge amount of funds, admin may listen and front run the transaction and set large fee value. User may not aware of this and thinking like the fee are less and tried to withdraw.

Tools Used

Manual review

Recommended Mitigation Steps

We suggest to set the cap on the fee . something like based on percentage basis.

c4-judge commented 1 year ago

JustDravee marked the issue as unsatisfactory: Out of scope