code-423n4 / 2023-02-kuma-findings

2 stars 1 forks source link

Bonds can be sold as soon as the contract is initialized hence bypassing the fees #31

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/main/src/kuma-protocol/KUMASwap.sol#L86 https://github.com/code-423n4/2023-02-kuma/blob/main/src/kuma-protocol/KUMASwap.sol#L359-L363

Vulnerability details

Impact

In KUMASwap contract, the setting of fees is done in a separate transaction after contract initialization. The fee variables are not set during initialization.

    function setFees(uint16 variableFee, uint256 fixedFee) external override onlyRole(Roles.KUMA_MANAGER_ROLE) {
        _variableFee = variableFee;
        _fixedFee = fixedFee;
        emit FeeSet(variableFee, fixedFee);
    }

The delay between contract initialization and setting up of fees can be exploited by anyone to sell the bonds without incurring protocol fees.

This essentially lets the attacker to escape fees that should have ideally been transferred to KUMAFeeCollector, causing a loss to protocol.

Proof of Concept

Consider this scenario:

This essentially lets the attacker to escape fees that should have ideally been transferred to KUMAFeeCollector, causing a loss to protocol.

Tools Used

Manual review

Recommended Mitigation Steps

Consider setting the fee variables in the initialize function itself.

GalloDaSballo commented 1 year ago

Looks like a gotcha mostly, am thinking QA

GalloDaSballo commented 1 year ago

See Discussion on #26

L

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

c4-sponsor commented 1 year ago

m19 marked the issue as sponsor acknowledged

m19 commented 1 year ago

This is technically true, most likely new KUMASwaps will be launched without a fee so in that case this was intended behavior. We agree that it could also be passed in the initialize instead.

We agree with the QA rating for this issue.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-c

GalloDaSballo commented 1 year ago

Finding is valid, but after scoring the QA Submissions the score is not sufficient