code-423n4/2024-08-phi-validation

When calculating the price, the cred can return an incorrect price after a fee.

c4-bot-5 opened this issue · 0 comments

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/main/src/curve/BondingCurve.sol#L141-L143

Vulnerability details

Vulnerability details

when calculating the price cred after the fee may return an incorrect price,
In the _getCreatorFee function that calculates the price taking into account the creator's fee, there is a condition that when supply ==0 the fee should also be equal to 0. The vulnerability is that because the function assigns creatorFee=0 but does not return it, so the function continues its execution. Regardless of the fact that supply ==0 creatorFee will be calculated as usual, thus confusing the curators.

 function _getCreatorFee(
       uint256 credId_,
       uint256 supply_,
       uint256 price_,
       bool isSign_
   )
       internal
       view
       returns (uint256 creatorFee)
   {
       if (!credContract.isExist(credId_)) {
           return 0;
       }
       if (supply_ == 0) {
           creatorFee = 0;
       }

       (uint16 buyShareRoyalty, uint16 sellShareRoyalty) = credContract.getCreatorRoyalty(credId_);

       uint16 royaltyRate = isSign_ ? buyShareRoyalty : sellShareRoyalty;
       creatorFee = (price_ * royaltyRate) / RATIO_BASE;
   }

Proof of Concept

Paste this test in Cred.t.sol.

function testPriceAfterFee() public {

_createCred("BASIC", "SIGNATURE", 0x0);

  

uint256 id = 1;

uint256 supply = 0;

uint256 amount = 1;

uint256 ratioBase = 10_000;

  

uint256 totalPrice = bondingCurve.getBuyPriceAfterFee(id, supply, amount);

uint256 price = bondingCurve.getBuyPrice(supply, amount);

uint256 protocolFee = price * cred.protocolFeePercent() / ratioBase;

uint256 fee = 0;

  

assertEq(totalPrice, price + protocolFee + fee);

}
forge test --mt testPriceAfterFee 

Ran 1 test for test/Cred.t.sol:TestCred
[FAIL. Reason: assertion failed: 1070610610610610 != 1060510510510510] testPriceAfterFee() (gas: 500934)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 18.58ms (6.04ms CPU time)

Ran 1 test suite in 241.63ms (18.58ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/Cred.t.sol:TestCred
[FAIL. Reason: assertion failed: 1070610610610610 != 1060510510510510] testPriceAfterFee() (gas: 500934)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

forge

Recommended Mitigation Steps

Add return to the condition

function _getCreatorFee(
        uint256 credId_,
        uint256 supply_,
        uint256 price_,
        bool isSign_
    )
        internal
        view
        returns (uint256 creatorFee)
    {
        if (!credContract.isExist(credId_)) {
            return 0;
        }
        if (supply_ == 0) {
-            creatorFee = 0;
+			 return creatorFee=0;
        }

        (uint16 buyShareRoyalty, uint16 sellShareRoyalty) = credContract.getCreatorRoyalty(credId_);

        uint16 royaltyRate = isSign_ ? buyShareRoyalty : sellShareRoyalty;
        creatorFee = (price_ * royaltyRate) / RATIO_BASE;
    }

Assessed type

Invalid Validation