code-423n4 / 2024-01-renft-findings

2 stars 0 forks source link

Users funds can be locked in escrow module due to possibility of feeNumerator being set too high #506

Closed c4-bot-6 closed 10 months ago

c4-bot-6 commented 10 months ago

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Admin.sol#L173

Vulnerability details

The setFee function in the Admin contract allows the admin to set the protocol fee numerator. However, there is no explicit check in the code to ensure that the feeNumerator does not exceed a certain maximum value, which is typically necessary to prevent setting fees that are too high.

function setFee(uint256 feeNumerator) external onlyRole("ADMIN_ADMIN") {
    ESCRW.setFee(feeNumerator);
}

Impact

Without a check on the feeNumerator, an admin could potentially set an excessively high fee, which could be detrimental to the users of the protocol. It could lead to a loss of trust, a decrease in usage, and an overall negative impact on the protocol's reputation and adoption. In the worst case, if the fee is set to 100% or higher, it could effectively lock users' funds within the escrow module.

Mitigations

function setFee(uint256 feeNumerator) external onlyRole("ADMIN_ADMIN") {
    require(feeNumerator <= 1000, "Admin: Fee cannot exceed 10%");
    ESCRW.setFee(feeNumerator);
}

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

141345 marked the issue as insufficient quality report

c4-judge commented 10 months ago

0xean marked the issue as unsatisfactory: Insufficient quality