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

3 stars 1 forks source link

Accounts blocked from deposits can ultimately mimic the exact state as a depositor #44

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/market/WildcatMarketToken.sol#L41

Vulnerability details

Proof of Concept

Invariant states the following:

Accounts which are blocked from deposits, or which do not have a credential on markets which require it for deposits, should never be able to mint market tokens.

However, a user who is blocked from depositing still remains able to receive transfers of tokens that another account has deposited, which objectively reaches an identical state within the protocol.

Add the following test to WildcatMarketToken.t.sol:

  function testTransferToBlockedAccount() external {
    parameters.hooksConfig = parameters.hooksConfig.setFlag(Bit_Enabled_Transfer);
    BaseMarketTest.setUpContracts(true);
    token = IERC20(address(market));
    MarketState memory state;
    _mint(bob, 1 ether);
    _mint(alice, 1 ether);
    //bob's credentials removed & blocked from depositing
    _blockLender(bob);
    vm.prank(alice);
    //alice transfers tokens to bob and state remains identical as if bob had deposited
    token.transfer(bob, 1 ether);
    vm.prank(bob);
    market.queueFullWithdrawal();
  }

Recommended Mitigation Steps

To fully enforce the stated invariant, a blocked depositor should not be able to receive market tokens.

Assessed type

Invalid Validation

laurenceday commented 1 month ago

See https://github.com/code-423n4/2024-08-wildcat-findings/issues/40#issuecomment-2388202441

3docSec commented 1 month ago

This check leaves little doubt that it's intended behavior:

File: AccessControlHooks.sol
862:     // If the recipient is a known lender, skip access control checks.
863:     if (!isKnownLenderOnMarket[to][msg.sender]) {

...and the invariant holds: tokens are transferred not minted

c4-judge commented 1 month ago

3docSec marked the issue as unsatisfactory: Invalid