Blacklisted Assets Can Be Used as Origination Fee Assets in Hooks Templates

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

The HooksFactory contract allows blacklisted assets to be set as origination fee assets when creating or updating hooks templates. This bypasses the intended blacklist restrictions, potentially enabling interaction with prohibited assets during market deployments. While the protocol controls the initial setting of the origination asset in templates, this vulnerability could lead to:

1: Regulatory compliance violations if blacklisted assets are used in market creations.

2: Undermining of the entire blacklist mechanism, reducing its effectiveness as a security measure.

3: Potential financial risks or loss of trust if markets are created using blacklisted assets.

4: Widespread impact, as multiple markets could be affected if created from a template with a blacklisted origination fee asset.

Proof of Concept

The following test demonstrates that a blacklisted asset can be successfully set as an origination fee asset in a hooks template:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.20;

import "forge-std/Test.sol";
import "../src/HooksFactory.sol";
import "../src/WildcatArchController.sol";

contract HooksFactoryBlacklistTest is Test {
    WildcatArchController public archController;
    HooksFactory public hooksFactory;
    address public owner;
    address public blacklistedAsset;
    address public hooksTemplate;

    function setUp() public {
        owner = address(this);
        blacklistedAsset = address(0x1);
        hooksTemplate = address(0x2);

        // Deploy ArchController
        archController = new WildcatArchController();

        // Deploy HooksFactory
        hooksFactory = new HooksFactory(
            address(archController),
            address(0), // sanctionsSentinel
            address(0), // marketInitCodeStorage
            0 // marketInitCodeHash
        );

        // Register HooksFactory as a controller factory
        vm.prank(archController.owner());
        archController.registerControllerFactory(address(hooksFactory));

        // Blacklist an asset
        vm.prank(archController.owner());
        archController.addBlacklist(blacklistedAsset);
    }

    function testBlacklistedOriginationFeeAsset() public {
        // Attempt to add a hooks template with a blacklisted origination fee asset
        vm.prank(archController.owner());

        // If this doesn't revert, it means the blacklist check is missing
        hooksFactory.addHooksTemplate(
            hooksTemplate,
            "Test Template",
            address(0x3), // feeRecipient
            blacklistedAsset, // originationFeeAsset
            100, // originationFeeAmount
            100 // protocolFeeBips
        );

        // If we reach here, the blacklisted asset was accepted
        HooksTemplate memory template = hooksFactory.getHooksTemplateDetails(hooksTemplate);

        assertTrue(template.exists, "Hooks template should exist");
        assertEq(
            template.originationFeeAsset, blacklistedAsset, "Blacklisted asset was accepted as originationFeeAsset"
        );
    }
}

Test Output

[PASS] testBlacklistedOriginationFeeAsset() (gas: 143370)
Traces:
  [143370] HooksFactoryBlacklistTest::testBlacklistedOriginationFeeAsset()
    ├─ [2361] WildcatArchController::owner() [staticcall]
    │   └─ ← [Return] HooksFactoryBlacklistTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]
    ├─ [0] VM::prank(HooksFactoryBlacklistTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   └─ ← [Return] 
    ├─ [117668] HooksFactory::addHooksTemplate(SHA-256: [0x0000000000000000000000000000000000000002], "Test Template", RIPEMD-160: [0x0000000000000000000000000000000000000003], ECRecover: [0x0000000000000000000000000000000000000001], 100, 100)
    │   ├─ [361] WildcatArchController::owner() [staticcall]
    │   │   └─ ← [Return] HooksFactoryBlacklistTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]
    │   ├─ emit HooksTemplateAdded(hooksTemplate: SHA-256: [0x0000000000000000000000000000000000000002], name: "Test Template", feeRecipient: RIPEMD-160: [0x0000000000000000000000000000000000000003], originationFeeAsset: ECRecover: [0x0000000000000000000000000000000000000001], originationFeeAmount: 100, protocolFeeBips: 100)
    │   └─ ← [Stop] 
    ├─ [2511] HooksFactory::getHooksTemplateDetails(SHA-256: [0x0000000000000000000000000000000000000002]) [staticcall]
    │   └─ ← [Return] HooksTemplate({ originationFeeAsset: 0x0000000000000000000000000000000000000001, originationFeeAmount: 100, protocolFeeBips: 100, exists: true, enabled: true, index: 0, feeRecipient: 0x0000000000000000000000000000000000000003, name: "Test Template" })
    ├─ [0] VM::assertTrue(true, "Hooks template should exist") [staticcall]
    │   └─ ← [Return] 
    ├─ [0] VM::assertEq(ECRecover: [0x0000000000000000000000000000000000000001], ECRecover: [0x0000000000000000000000000000000000000001], "Blacklisted asset was accepted as originationFeeAsset") [staticcall]
    │   └─ ← [Return] 
    └─ ← [Stop]

The test passes, confirming that a blacklisted asset (address 0x1) was successfully set as the origination fee asset in a new hooks template.

Tools Used

Manual review

Recommended Mitigation Steps

1: Modify the addHooksTemplate function in the HooksFactory contract to include a blacklist check:

function addHooksTemplate(
    address hooksTemplate,
    string calldata name,
    address feeRecipient,
    address originationFeeAsset,
    uint80 originationFeeAmount,
    uint16 protocolFeeBips
) external override onlyArchControllerOwner {
    // ... existing checks ...

    if (IWildcatArchController(_archController).isBlacklistedAsset(originationFeeAsset)) {
        revert AssetBlacklisted();
    }

    // ... rest of the function ...
}

2: Implement a mechanism to periodically check and potentially disable templates that use assets that have become blacklisted after template creation.

Assessed type

Other

[laurenceday]

Not a valid finding.

The asset blacklist is intended to cover assets that are problematic and we don't want people to create markets over for technical reasons (i.e. rebasing assets such as stETH due to how it breaks the underlying interest rate model), or ones which legal guidance suggests that it isn't prudent for Wildcat markets permitting credit denominated in them to exist.

With that said, the status of these assets on the blacklist has no bearing on whether or not they can be used as payment assets via origination fee. We can accept stETH no problems.

[d1ll0n]

Also worth noting that the arch controller owner sets both the origination fee asset and the blacklist, so they could override it regardless.

[3docSec]

Intended behavior as per sponsor's comment. Also worth noting: even if it wasn't, the root cause is missing validation on admin input (onlyArchControllerOwner), so the finding would've been QA at best

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid