code-423n4 / 2024-06-panoptic-findings

1 stars 0 forks source link

The `startToken` function in the `CollateralTracker` contract is missing a critical modifier to ensure that only the associated Panoptic pool can call it #36

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/CollateralTracker.sol#L210-L249

Vulnerability details

Impact

Detailed description of the impact of this finding.

Without the onlyPanopticPool modifier, any external entity can call startToken, potentially allowing unauthorized initialization of the collateral tracker. This could lead to incorrect tracking of collateral, manipulation of pool assets, and unauthorized changes to the state variables.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

The startToken function is intended to be called once by the factory to initialize the collateral tracking system. However, as it is currently implemented without the onlyPanopticPool modifier, it can be called by any address:

function startToken(
    bool underlyingIsToken0,
    address token0,
    address token1,
    uint24 fee,
    PanopticPool panopticPool
) external {
    // Initialization code...
}

An attacker could potentially call this function with false parameters, leading to a misconfigured collateral tracker.

POC

File Name: test/foundry/core/CollateralTracker.t.sol
Prerequisite: Insert the test function beneath into line 597 just after the prior function in the CollateralTracker.t.sol file.
Then open terminal> CD into the panoptic root folder> and run: forge test -vvvvv --match-test test_Success_Unauth_StartToken_virtualShares  --fork-url "https://eth-mainnet.g.alchemy.com/v2/{Token}"
function test_Success_Unauth_StartToken_virtualShares() public {
        _initWorld(0);
        CollateralTracker ct = new CollateralTracker(
            10,
            2_000,
            1_000,
            -1_024,
            5_000,
            9_000,
            20_000
        );

        vm.prank(address(0xbEEF));
        ct.startToken(false, token0, token1, fee, panopticPool);

        assertEq(ct.totalSupply(), 10 ** 6);
        assertEq(ct.totalAssets(), 1);
    }
forge test -vvvvv --match-test test_Success_Unauth_StartToken_virtualShares  --fork-url "https://eth-mainnet.g.alchemy.com/v2/{Token}"
[⠊] Compiling...
[⠢] Compiling 1 files with Solc 0.8.25
[⠆] Solc 0.8.25 finished in 10.56s
Compiler run successful with warnings:

Ran 1 test for test/foundry/core/CollateralTracker.t.sol:CollateralTrackerTest
[PASS] test_Success_Unauth_StartToken_virtualShares() (gas: 48849426)
Logs:
  Bound Result 0

Traces:
  [230] CollateralTrackerTest::setUp()
    └─ ← [Stop] 

  [48849426] CollateralTrackerTest::test_Success_Unauth_StartToken_virtualShares()
    ├─ [0] console::log("Bound Result", 0) [staticcall]
    │   └─ ← [Stop] 
    ├─ [279] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::tickSpacing() [staticcall]
    │   └─ ← [Return] 10
    ├─ [266] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::token0() [staticcall]
    │   └─ ← [Return] 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48
    ├─ [308] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::token1() [staticcall]
    │   └─ ← [Return] 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2
    ├─ [251] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::fee() [staticcall]
    │   └─ ← [Return] 500
    ├─ [279] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::tickSpacing() [staticcall]
    │   └─ ← [Return] 10
    ├─ [2696] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::slot0() [staticcall]
    │   └─ ← [Return] 1306562736910707301497021609837803 [1.306e33], 194221 [1.942e5], 720, 723, 723, 0, true
    ├─ [2364] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::feeGrowthGlobal0X128() [staticcall]
    │   └─ ← [Return] 2944782450825845311516891440980176 [2.944e33]
    ├─ [2388] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::feeGrowthGlobal1X128() [staticcall]
    │   └─ ← [Return] 1359677158233598538395938809009518466128500 [1.359e42]
    ├─ [3591883] → new SemiFungiblePositionManagerHarness@0x2b42C737b072481672Bb458260e9b59CB2268dc6
    │   └─ ← [Return] 17938 bytes of code
    ├─ [53716] SemiFungiblePositionManagerHarness::initializeAMMPool(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, 500)
    │   ├─ [2666] 0x1F98431c8aD98523631AE4a59f267346ea31F984::getPool(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, 500) [staticcall]
    │   │   └─ ← [Return] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640
    │   ├─ [279] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::tickSpacing() [staticcall]
    │   │   └─ ← [Return] 10
    │   ├─ emit PoolInitialized(uniswapPool: 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640, poolId: 2965273888087506 [2.965e15])
    │   └─ ← [Stop] 
    ├─ [2190479] → new PanopticHelper@0x6187F206E5b64D97E5136B5779683a923EaEB1B4
    │   └─ ← [Return] 10939 bytes of code
    ├─ [16380118] → new PanopticPoolHarness@0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3
    │   └─ ← [Return] 81758 bytes of code
    ├─ [22882700] PanopticPoolHarness::modifiedStartPool(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640)
    │   ├─ [266] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::token0() [staticcall]
    │   │   └─ ← [Return] 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48
    │   ├─ [308] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::token1() [staticcall]
    │   │   └─ ← [Return] 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2
    │   ├─ [11111903] → new CollateralTrackerHarness@0xAE11e8F5032795b2E17418cfa12B8B4260A2084F
    │   │   └─ ← [Return] 55255 bytes of code
    │   ├─ [11111903] → new CollateralTrackerHarness@0xdC9E67dF42af6FeB30BE1Cf48af46234Ceebef85
    │   │   └─ ← [Return] 55255 bytes of code
    │   ├─ [251] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::fee() [staticcall]
    │   │   └─ ← [Return] 500
    │   ├─ [157934] CollateralTrackerHarness::startToken(true, 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, 500, PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3])
    │   │   └─ ← [Stop] 
    │   ├─ [251] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::fee() [staticcall]
    │   │   └─ ← [Return] 500
    │   ├─ [157944] CollateralTrackerHarness::startToken(false, 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, 500, PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3])
    │   │   └─ ← [Stop] 
    │   ├─ [696] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::slot0() [staticcall]
    │   │   └─ ← [Return] 1306562736910707301497021609837803 [1.306e33], 194221 [1.942e5], 720, 723, 723, 0, true
    │   ├─ [43653] PanopticMath::computeMedianObservedPrice() [delegatecall]
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(720) [staticcall]
    │   │   │   └─ ← [Return] 1717786115 [1.717e9], 19445127011411 [1.944e13], 152409937106708242648556935919 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(715) [staticcall]
    │   │   │   └─ ← [Return] 1717786043 [1.717e9], 19445113028231 [1.944e13], 152409935041071471549432686884 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(710) [staticcall]
    │   │   │   └─ ← [Return] 1717785923 [1.717e9], 19445089720355 [1.944e13], 152409931356215014535371837161 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(705) [staticcall]
    │   │   │   └─ ← [Return] 1717785839 [1.717e9], 19445073404171 [1.944e13], 152409928750996661148836034758 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(700) [staticcall]
    │   │   │   └─ ← [Return] 1717785695 [1.717e9], 19445045433395 [1.944e13], 152409924277356282396678726847 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(695) [staticcall]
    │   │   │   └─ ← [Return] 1717785599 [1.717e9], 19445026786523 [1.944e13], 152409921293595660264102857309 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(690) [staticcall]
    │   │   │   └─ ← [Return] 1717785503 [1.717e9], 19445008138535 [1.944e13], 152409918289529672036686681798 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(685) [staticcall]
    │   │   │   └─ ← [Return] 1717785407 [1.717e9], 19444989490979 [1.944e13], 152409915282596134171926629340 [1.524e29], true
    │   │   ├─ [2635] 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640::observations(680) [staticcall]
    │   │   │   └─ ← [Return] 1717785323 [1.717e9], 19444973174783 [1.944e13], 152409912665528661362616735012 [1.524e29], true
    │   │   └─ ← [Return] 194240 [1.942e5]
    │   ├─ [33962] 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48::approve(SemiFungiblePositionManagerHarness: [0x2b42C737b072481672Bb458260e9b59CB2268dc6], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   ├─ [26673] 0x43506849D7C04F9138D1A2050bbF3A0c054402dd::approve(SemiFungiblePositionManagerHarness: [0x2b42C737b072481672Bb458260e9b59CB2268dc6], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) [delegatecall]
    │   │   │   ├─ emit Approval(owner: PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3], spender: SemiFungiblePositionManagerHarness: [0x2b42C737b072481672Bb458260e9b59CB2268dc6], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   │   └─ ← [Return] true
    │   │   └─ ← [Return] true
    │   ├─ [24420] 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2::approve(SemiFungiblePositionManagerHarness: [0x2b42C737b072481672Bb458260e9b59CB2268dc6], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   ├─ emit Approval(owner: PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3], spender: SemiFungiblePositionManagerHarness: [0x2b42C737b072481672Bb458260e9b59CB2268dc6], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   └─ ← [Return] true
    │   ├─ [25462] 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48::approve(CollateralTrackerHarness: [0xAE11e8F5032795b2E17418cfa12B8B4260A2084F], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   ├─ [24673] 0x43506849D7C04F9138D1A2050bbF3A0c054402dd::approve(CollateralTrackerHarness: [0xAE11e8F5032795b2E17418cfa12B8B4260A2084F], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) [delegatecall]
    │   │   │   ├─ emit Approval(owner: PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3], spender: CollateralTrackerHarness: [0xAE11e8F5032795b2E17418cfa12B8B4260A2084F], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   │   └─ ← [Return] true
    │   │   └─ ← [Return] true
    │   ├─ [24420] 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2::approve(CollateralTrackerHarness: [0xdC9E67dF42af6FeB30BE1Cf48af46234Ceebef85], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   ├─ emit Approval(owner: PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3], spender: CollateralTrackerHarness: [0xdC9E67dF42af6FeB30BE1Cf48af46234Ceebef85], amount: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    │   │   └─ ← [Return] true
    │   └─ ← [Stop] 
    ├─ [619] PanopticPoolHarness::collateralToken0() [staticcall]
    │   └─ ← [Return] CollateralTrackerHarness: [0xAE11e8F5032795b2E17418cfa12B8B4260A2084F]
    ├─ [454] PanopticPoolHarness::collateralToken1() [staticcall]
    │   └─ ← [Return] CollateralTrackerHarness: [0xdC9E67dF42af6FeB30BE1Cf48af46234Ceebef85]
    ├─ [3106121] → new CollateralTracker@0xafc8A7F61B9E656281b9Eff091641CdDAb8bE9ac
    │   └─ ← [Return] 15510 bytes of code
    ├─ [0] VM::prank(0x000000000000000000000000000000000000bEEF)
    │   └─ ← [Return] 
    ├─ [156984] CollateralTracker::startToken(false, 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, 500, PanopticPoolHarness: [0x25690f2BCf09D7e1286Fc35378433CfD3006E7C3])
    │   └─ ← [Stop] 
    ├─ [416] CollateralTracker::totalSupply() [staticcall]
    │   └─ ← [Return] 1000000 [1e6]
    ├─ [339] CollateralTracker::totalAssets() [staticcall]
    │   └─ ← [Return] 1
    └─ ← [Stop] 

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.04s (5.63s CPU time)

Ran 1 test suite in 8.31s (7.04s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual review and Foundry for POC.

Recommended Mitigation Steps

To mitigate this issue, the startToken function should include the onlyPanopticPool modifier to restrict access to the associated Panoptic pool:

function startToken(
    bool underlyingIsToken0,
    address token0,
    address token1,
    uint24 fee,
    PanopticPool panopticPool
) external onlyPanopticPool {
    // Initialization code...
}

Additionally, consider implementing role-based access control (RBAC) to manage permissions more granularly and securely.

Assessed type

Access Control

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 1 month ago

Called atomically during the deployment