Protocol and referral fee would be permanently stuck in the Curves contract when selling a token

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L231

Vulnerability details

Impact

Protocol fee and referral fee retrieved during the sale of a token stay permanently stuck in the Curves contract.

Proof of Concept

• During the sale of a token Curves._transferFee subtracts all the fees from the selling price and transfers the remaining to the seller: https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L231

      uint256 sellValue = price - protocolFee - subjectFee - referralFee - holderFee;
      (bool success1, ) = firstDestination.call{value: isBuy ? buyValue : sellValue}("");

• However, the protocolFee taken away is not transferred to the protocolFeeDestination in the remainder of the _transferFee function: https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L231-L250. It stays back in the contract with no other of way of retrieval.
• Further, referralFee is taken away without checking if a referral address actually exists. In the event that there is no referral defined, the third transfer is never executed: https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L240. This leaves the referral fee in the contract, again no retrieval mechanism.

  Tools Used

Manual review

Recommended Mitigation Steps

• The buyValue (https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L230C59-L230C59) variable already has the logic for jointly handling the protocol and referral fee.

• It can be given a more generic name, and be transferred to the protocolDestination for both buying and selling transactions.

  uint256 protocolShare = referralDefined ? protocolFee : protocolFee + referralFee;
  uint256 sellValue = price - protocolFee - subjectFee - referralFee - holderFee;
  
  (bool success, ) = (feesEconomics.protocolFeeDestination).call{value: protocolShare}("");
  if (!success) revert CannotSendFunds();
  if(!isBuy) {
      (bool success1, ) = (msg.sender).call{value: sellValue}("");
      if(!success1) revert CannotSendFunds();
  }

Assessed type

ETH-Transfer

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #230

[c4-judge]

alcueca marked the issue as selected for report

[c4-judge]

alcueca changed the severity to 2 (Med Risk)

[c4-judge]

alcueca marked the issue as satisfactory

[c4-sponsor]

andresaiello (sponsor) acknowledged