code-423n4 / 2024-08-wildcat-findings

3 stars 1 forks source link

Improper validation on the `HooksFactory::_validateFees` function precisely on the `OriginationFee`and `ProtocolFeeBips` parameters, Leading to a borrower deploying a market without paying the Origination Fee. #56

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/HooksFactory.sol#L153-L170

Vulnerability details

Proof of Concept

In the wildcat protocol the archController adds a hook template with some key parameters address hooksTemplate,string calldata name, address feeRecipient,address originationFeeAsset, uint80 originationFeeAmount, uint16 protocolFeeBips, on function HooksFactory::addHooksTemplate , the function _validateFees does the validition of fees without a proper validation check on originationFeeAmount and protocolFeeBips. When an ArchController inputs originationFeeAmount equal to 0 it will not revert.Leading to a hook template deployed with an originationFeeAmount of 0. That is when it becomes tricky since the borrower can now deploy a market without paying an originationFee in the HooksFactory::deployMarket function .


if (originationFeeAsset != address(0)) {
      originationFeeAsset.safeTransferFrom(
        msg.sender,
        templateDetails.feeRecipient,
        originationFeeAmount
      );
    }

Proof Of Code

function test_deployMarketWithoutOriginationFeeAmount() external {
    // added a valid underlying token
    hooksFactory.addHooksTemplate(hooksTemplate, 'template', address(1), address(underlying), 0, 0);
    archController.registerBorrower(address(this));

    bytes memory constructorArgs = '';
    MockHooks hooksInstance = _validateDeployHooksInstance(hooksTemplate, constructorArgs);

    bytes memory createMarketHooksData = 'o hey this is my createMarketHooksData do u like it';

    DeployMarketInputs memory parameters = DeployMarketInputs({
      asset: address(underlying),
      namePrefix: 'name',
      symbolPrefix: 'symbol',
      maxTotalSupply: type(uint128).max,
      annualInterestBips: 1000,
      delinquencyFeeBips: 1000,
      withdrawalBatchDuration: 10000,
      reserveRatioBips: 10000,
      delinquencyGracePeriod: 10000,
      hooks: EmptyHooksConfig.setHooksAddress(address(hooksInstance))
    });

    //the borrower deploys the market with 0 fees
    address market = hooksFactory.deployMarket(
      parameters,
      createMarketHooksData,
      bytes32(uint(1)),
      address(underlying),
      0
    );
  }

Impact

The borrower will deploy a market without paying any originationFeeAmount and protocolFeeBips leading to a loss of funds for the protocol.

Tools Used

Manual Review.

Recommended Mitigation Steps

Ensure that theres is a check on originationFeeAmount and protocolFeeBips that they are not equal to to 0 else they should revert.

if (
      (protocolFeeBips > 0 && nullFeeRecipient) ||
      (hasOriginationFee && nullFeeRecipient) ||
      (hasOriginationFee && nullOriginationFeeAsset) ||
      protocolFeeBips > 1_000 ||
+      originationFeeAmount == 0 ||
+      protocolFeeBips == 0
    ) {
      revert InvalidFeeConfiguration();
    }

Assessed type

Invalid Validation

d1ll0n commented 1 month ago

If the originationFeeAmount is zero, there's no fee to pay - fees aren't required for every template. The protocol fee: can also be zero; is on the interest not a flat token amount; can be updated.

3docSec commented 1 month ago

Invalidating - intended behavior

c4-judge commented 1 month ago

3docSec marked the issue as unsatisfactory: Invalid