search

code-423n4 / 2023-09-centrifuge-findings

16 stars 14 forks source link

[M-01] Multiple calls are executed in the same transaction within the Factory contract #18

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/util/Factory.sol#L114

Vulnerability details

Impact

Detailed description of the impact of this finding. This call is executed following another call within the same transaction. It is possible that the call never gets executed if a prior call fails permanently. This might be caused intentionally by a malicious callee. The vulnerability is in the factory contract within the newTrancheToken function.

Gas Exhaustion Vulnerability: Smart contracts on the Ethereum blockchain rely on gas to execute transactions and contract functions. Attackers can craft transactions that consume an excessive amount of gas, either by executing complex computations or through recursive calls. This results in a high transaction fee, making it costly for users to interact with the contract or even preventing them from doing so.

External Contract Calls Vulnerability: Contracts may rely on external data or interact with other contracts. If an external contract is compromised or becomes unavailable, it can lead to a DoS condition.

Proof of Concept

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

// https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/util/Factory.sol#L114
        restrictionManager.updateMember(RootLike(root).escrow(), type(uint256).max);

Test Case Foundry

1. Paste code below into the test/util/Factory.t.sol contract.
2. In terminal run: forge test -vvvvv --match-path "test/util/Factory.t.sol" --match-test "testDos"
3. Dos is actioned and test does not revert.

Exploit Foundry

function testDos(        
        bytes32 salt,
        uint64 poolId,
        bytes16 trancheId,
        address investmentManager,
        address poolManager,
        string memory name,
        string memory symbol,
        uint8 decimals
        ) public payable {
        address predictedAddress = address(
            uint160(
                uint256(
                    keccak256(
                        abi.encodePacked(
                            bytes1(0xff),
                            address(this),
                            salt,
                            keccak256(abi.encodePacked(type(TrancheTokenFactory).creationCode, abi.encode(root)))
                        )
                    )
                )
            )
        );

        address[] memory trancheTokenWards = new address[](2);
        trancheTokenWards[0] = address(investmentManager);
        trancheTokenWards[1] = address(poolManager);

        address[] memory memberlistWards = new address[](1);
        memberlistWards[0] = address(poolManager);

        TrancheTokenFactory trancheTokenFactory = new TrancheTokenFactory{ salt: salt }(root);
        assertEq(address(trancheTokenFactory), predictedAddress);
        trancheTokenFactory.newTrancheToken(
            poolId, trancheId, name, symbol, decimals, trancheTokenWards, memberlistWards
        );
        vm.expectRevert();
        trancheTokenFactory.newTrancheToken(0, bytes16("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), name , symbol , 0, (trancheTokenWards), (memberlistWards));
}

Log

Running 1 test for test/util/Factory.t.sol:FactoryTest
[FAIL. Reason: Call did not revert as expected Counterexample: calldata=0x425ddf4b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005691e42f302d7d527851fc18dd210000000000000000000000000de594a49cdf7afd00729578022b480f25b63e5b00000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000140000000000000000000000000000000000000000000000000000000000000001c000000000000000000000000000000000000000000000000000000000000000bfdb7a876f1ab7721043f840000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024af3ae3093b43afc838967f40328cd68ad0db22fdd62e0b9171208b765e7e2c51e7e847ef00000000000000000000000000000000000000000000000000000000, args=[0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0x00000000000000000000000000000000, 0x0000000000005691E42f302d7D527851Fc18dd21, 0x0de594A49CdF7AFd00729578022b480f25b63E5b, ���v�w!?�, �:�     ;C��8�@2�֊��"��.
                                                                                                �q �v^~,Q��G�, 28]] testDos(bytes32,uint64,bytes16,address,address,string,string,uint8) (runs: 0, μ: 0, ~: 0)
Traces:
  [922188] FactoryTest::setUp() 
    ├─ [256995] → new Escrow@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
    │   ├─ emit Rely(user: FactoryTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   └─ ← 1167 bytes of code
    ├─ [577955] → new Root@0x2e234DAe75C793f67A35089C9d99245E1C58470b
    │   ├─ emit Rely(user: FactoryTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   └─ ← 2537 bytes of code
    └─ ← ()

  [8345462] FactoryTest::testDos(0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0x00000000000000000000000000000000, 0x0000000000005691E42f302d7D527851Fc18dd21, 0x0de594A49CdF7AFd00729578022b480f25b63E5b, ���v�w!?�, �:�  ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�, 28) 
    ├─ [3039487] → new TrancheTokenFactory@0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b
    │   ├─ emit Rely(user: FactoryTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   └─ ← 15063 bytes of code
    ├─ [2587986] TrancheTokenFactory::newTrancheToken(0, 0x00000000000000000000000000000000, ���v�w!?�, �:�     ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�, 28, [0x0000000000005691E42f302d7D527851Fc18dd21, 0x0de594A49CdF7AFd00729578022b480f25b63E5b], [0x0de594A49CdF7AFd00729578022b480f25b63E5b]) 
    │   ├─ [455188] → new RestrictionManager@0x43B4E43B6509B058488DD094606C05D905A6adE7
    │   │   ├─ emit Rely(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← 2157 bytes of code
    │   ├─ [282] Root::escrow() [staticcall]
    │   │   └─ ← Escrow: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]
    │   ├─ [22758] RestrictionManager::updateMember(Escrow: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) 
    │   │   └─ ← ()
    │   ├─ [23882] RestrictionManager::rely(Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) 
    │   │   ├─ emit Rely(user: Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
    │   │   └─ ← ()
    │   ├─ [23882] RestrictionManager::rely(0x0de594A49CdF7AFd00729578022b480f25b63E5b) 
    │   │   ├─ emit Rely(user: 0x0de594A49CdF7AFd00729578022b480f25b63E5b)
    │   │   └─ ← ()
    │   ├─ [1506] RestrictionManager::deny(TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b]) 
    │   │   ├─ emit Deny(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← ()
    │   ├─ [1826373] → new TrancheToken@0x13E8748E9B4653e422D6003D76e2D90D9978d54e
    │   │   ├─ emit Rely(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← 8978 bytes of code
    │   ├─ [24008] TrancheToken::file(0x6e616d6500000000000000000000000000000000000000000000000000000000, ���v�w!?�) 
    │   │   ├─ emit File(what: 0x6e616d6500000000000000000000000000000000000000000000000000000000, data: ���v�w!?�)
    │   │   └─ ← ()
    │   ├─ [70751] TrancheToken::file(0x73796d626f6c0000000000000000000000000000000000000000000000000000, �:�   ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�) 
    │   │   ├─ emit File(what: 0x73796d626f6c0000000000000000000000000000000000000000000000000000, data: �:�    ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�)
    │   │   └─ ← ()
    │   ├─ [24517] TrancheToken::file(0x7265737472696374696f6e4d616e616765720000000000000000000000000000, RestrictionManager: [0x43B4E43B6509B058488DD094606C05D905A6adE7]) 
    │   │   ├─ emit File(what: 0x7265737472696374696f6e4d616e616765720000000000000000000000000000, addr: RestrictionManager: [0x43B4E43B6509B058488DD094606C05D905A6adE7])
    │   │   └─ ← ()
    │   ├─ [24211] TrancheToken::rely(Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) 
    │   │   ├─ emit Rely(user: Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
    │   │   └─ ← ()
    │   ├─ [24211] TrancheToken::rely(0x0000000000005691E42f302d7D527851Fc18dd21) 
    │   │   ├─ emit Rely(user: 0x0000000000005691E42f302d7D527851Fc18dd21)
    │   │   └─ ← ()
    │   ├─ [24211] TrancheToken::rely(0x0de594A49CdF7AFd00729578022b480f25b63E5b) 
    │   │   ├─ emit Rely(user: 0x0de594A49CdF7AFd00729578022b480f25b63E5b)
    │   │   └─ ← ()
    │   ├─ [1788] TrancheToken::deny(TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b]) 
    │   │   ├─ emit Deny(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← ()
    │   └─ ← TrancheToken: [0x13E8748E9B4653e422D6003D76e2D90D9978d54e]
    ├─ [0] VM::expectRevert() 
    │   └─ ← ()
    ├─ [2585486] TrancheTokenFactory::newTrancheToken(0, 0x00000000000000000000000000000000, ���v�w!?�, �:�     ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�, 0, [0x0000000000005691E42f302d7D527851Fc18dd21, 0x0de594A49CdF7AFd00729578022b480f25b63E5b], [0x0de594A49CdF7AFd00729578022b480f25b63E5b]) 
    │   ├─ [455188] → new RestrictionManager@0x4D072967707fc78f59E547ed8148CC3886e8Ad35
    │   │   ├─ emit Rely(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← 2157 bytes of code
    │   ├─ [282] Root::escrow() [staticcall]
    │   │   └─ ← Escrow: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]
    │   ├─ [22758] RestrictionManager::updateMember(Escrow: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]) 
    │   │   └─ ← ()
    │   ├─ [23882] RestrictionManager::rely(Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) 
    │   │   ├─ emit Rely(user: Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
    │   │   └─ ← ()
    │   ├─ [23882] RestrictionManager::rely(0x0de594A49CdF7AFd00729578022b480f25b63E5b) 
    │   │   ├─ emit Rely(user: 0x0de594A49CdF7AFd00729578022b480f25b63E5b)
    │   │   └─ ← ()
    │   ├─ [1506] RestrictionManager::deny(TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b]) 
    │   │   ├─ emit Deny(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← ()
    │   ├─ [1826373] → new TrancheToken@0x80911DD6826B897dA9A03bcE19D1f7f503B95b81
    │   │   ├─ emit Rely(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← 8978 bytes of code
    │   ├─ [24008] TrancheToken::file(0x6e616d6500000000000000000000000000000000000000000000000000000000, ���v�w!?�) 
    │   │   ├─ emit File(what: 0x6e616d6500000000000000000000000000000000000000000000000000000000, data: ���v�w!?�)
    │   │   └─ ← ()
    │   ├─ [70751] TrancheToken::file(0x73796d626f6c0000000000000000000000000000000000000000000000000000, �:�   ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�) 
    │   │   ├─ emit File(what: 0x73796d626f6c0000000000000000000000000000000000000000000000000000, data: �:�    ;C��8�@2�֊��"��.
                                                                                                                                �q �v^~,Q��G�)
    │   │   └─ ← ()
    │   ├─ [24517] TrancheToken::file(0x7265737472696374696f6e4d616e616765720000000000000000000000000000, RestrictionManager: [0x4D072967707fc78f59E547ed8148CC3886e8Ad35]) 
    │   │   ├─ emit File(what: 0x7265737472696374696f6e4d616e616765720000000000000000000000000000, addr: RestrictionManager: [0x4D072967707fc78f59E547ed8148CC3886e8Ad35])
    │   │   └─ ← ()
    │   ├─ [24211] TrancheToken::rely(Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) 
    │   │   ├─ emit Rely(user: Root: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
    │   │   └─ ← ()
    │   ├─ [24211] TrancheToken::rely(0x0000000000005691E42f302d7D527851Fc18dd21) 
    │   │   ├─ emit Rely(user: 0x0000000000005691E42f302d7D527851Fc18dd21)
    │   │   └─ ← ()
    │   ├─ [24211] TrancheToken::rely(0x0de594A49CdF7AFd00729578022b480f25b63E5b) 
    │   │   ├─ emit Rely(user: 0x0de594A49CdF7AFd00729578022b480f25b63E5b)
    │   │   └─ ← ()
    │   ├─ [1788] TrancheToken::deny(TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b]) 
    │   │   ├─ emit Deny(user: TrancheTokenFactory: [0xF6485eb17f3df45c0F6D5572dC91Fab87591f44b])
    │   │   └─ ← ()
    │   └─ ← TrancheToken: [0x80911DD6826B897dA9A03bcE19D1f7f503B95b81]
    └─ ← "Call did not revert as expected"

Test result: FAILED. 0 passed; 1 failed; finished in 359.56ms

Failing tests:
Encountered 1 failing test in test/util/Factory.t.sol:FactoryTest
[FAIL. Reason: Call did not revert as expected Counterexample: calldata=0x425ddf4b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005691e42f302d7d527851fc18dd210000000000000000000000000de594a49cdf7afd00729578022b480f25b63e5b00000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000140000000000000000000000000000000000000000000000000000000000000001c000000000000000000000000000000000000000000000000000000000000000bfdb7a876f1ab7721043f840000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024af3ae3093b43afc838967f40328cd68ad0db22fdd62e0b9171208b765e7e2c51e7e847ef00000000000000000000000000000000000000000000000000000000, args=[0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0x00000000000000000000000000000000, 0x0000000000005691E42f302d7D527851Fc18dd21, 0x0de594A49CdF7AFd00729578022b480f25b63E5b, ���v�w!?�, �:�     ;C��8�@2�֊��"��.
                                                                                                �q �v^~,Q��G�, 28]] testDos(bytes32,uint64,bytes16,address,address,string,string,uint8) (runs: 0, μ: 0, ~: 0)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Foundry. VS Code. Mythx.

Recommended Mitigation Steps

If possible, refactor the code such that each transaction only executes one external call or make sure that all callees can be trusted (i.e. they're part of your own codebase).

Mitigations: Use Trusted Contracts: Only interact with well-audited and trusted contracts. Fallback Mechanisms: Implement fallback mechanisms or time-based fallbacks to handle unresponsive external contracts. Emergency Stop: Consider implementing an emergency stop mechanism that allows contract owners to pause contract functions in case of an emergency.

Gas Limit Monitoring: Developers should monitor the gas consumption of critical contract functions and set appropriate gas limits. Gas Price Monitoring: Keep an eye on the gas prices in the Ethereum network and adjust gas limits accordingly. Limit Loop Iterations: Avoid unbounded loops within contract functions to prevent gas exhaustion.

Assessed type

DoS

raymondfam commented 1 year ago

Inadequate elaboration.

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as primary issue

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #115

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid