code-423n4 / 2024-04-gondi-findings

0 stars 0 forks source link

Function `addNewTranche()` should use `protocolFee` from `Loan` struct #65

Open c4-bot-7 opened 7 months ago

c4-bot-7 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/loans/MultiSourceLoan.sol#L377

Vulnerability details

Impact

The protocol fee value is recorded and stored in the Loan struct when a new loan is issued. However, when adding a new tranche, the function uses the current value of protocolFee.fraction instead of the value stored in the Loan struct. This could result in inconsistencies in fee collection, as the protocol fee value might be updated by the admin, while the value stored in the Loan struct remains unchanged.

if (_renegotiationOffer.fee > 0) {
    /// @dev Cached
    ProtocolFee memory protocolFee = _protocolFee;
    ERC20(_loan.principalAddress).safeTransferFrom(
        _renegotiationOffer.lender,
        protocolFee.recipient,
        _renegotiationOffer.fee.mulDivUp(protocolFee.fraction, _PRECISION) // @audit Use protocolFee from Loan instead
    );
}

Proof of Concept

The protocol fee value is stored in the Loan struct when a new loan is opened.

function _processOffersFromExecutionData(
  ...
) ... {
  Loan memory loan = Loan(
      _borrower,
      _tokenId,
      _nftCollateralAddress,
      _principalAddress,
      totalAmount,
      block.timestamp,
      _duration,
      tranche,
      protocolFee.fraction
  );
}

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using _loan.protocolFee instead of protocolFee.fraction in the addNewTranche() function.

Assessed type

Other

c4-judge commented 7 months ago

0xA5DF marked the issue as primary issue

c4-judge commented 7 months ago

0xA5DF marked the issue as selected for report

c4-judge commented 7 months ago

0xA5DF marked the issue as satisfactory

0xend commented 7 months ago

https://github.com/pixeldaogg/florida-contracts/pull/363