Lack of explicit initialization checks in the constructor. contract initialization.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/InvestmentManager.sol#L88-L94

Vulnerability details

Impact

In the InvestmentManager, there's a vulnerability related to contract initialization, has been identified. This vulnerability poses a significant security risk to the contract and its users. The issue revolves around the absence of explicit initialization checks in the constructor, making it susceptible to exploitation.

Proof of Concept

The vulnerability arises due to the reliance on the escrow and userEscrow addresses as indicators of contract initialization, without an accompanying boolean flag to explicitly confirm the initialization process. The constructor, shown below, initializes these two addresses but lacks a clear initialization check:

Code Link

constructor(address escrow_, address userEscrow_) {
    escrow = EscrowLike(escrow_);
    userEscrow = UserEscrowLike(userEscrow_);

    wards[msg.sender] = 1;
    emit Rely(msg.sender);
}

Exploitation Scenario

1. An attacker deploys the InvestmentManager contract with malicious intentions.

2. The attacker sets the escrow and userEscrow addresses to their own malicious contract addresses, which are not compliant with the actual EscrowLike and UserEscrowLike interfaces.

InvestmentManager manager = new InvestmentManager(address(new MaliciousEscrow()), address(new MaliciousUserEscrow()));

3. Since the constructor doesn't include an explicit initialized flag, the attacker successfully deploys the contract without any issues.

4. The attacker then calls sensitive functions on the manager contract, assuming that it's initialized. For example, they may call a function that transfers funds or executes other critical operations.

function exploitManager() public {
    manager.transferFundsTo(address(this)); // Assuming there's a vulnerable function like this
}

5. The exploitManager function exploits the contract, as it doesn't check for proper initialization using an explicit initialized flag. The attacker can potentially execute malicious code or drain funds, causing unexpected behavior or financial losses.

Tools Used

Vs

Recommended Mitigation Steps

It is crucial to include an initialized flag within the contract. This flag should only be set to true once all necessary initialization steps are successfully completed. Sensitive functions in the contract should then check the state of this flag before executing to ensure that the contract is in the desired initialized state.

For example:

bool public initialized;

constructor(address escrow_, address userEscrow_) {
    escrow = EscrowLike(escrow_);
    userEscrow = UserEscrowLike(userEscrow_);

    wards[msg.sender] = 1;
    emit Rely(msg.sender);

    initialized = true; // Mark the contract as initialized
}

This way, other functions in the contract can check the initialized flag to ensure that the contract is fully initialized before allowing sensitive operations.

Assessed type

Error

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #29

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid