Input data vulnerability enables unauthorized access and code injection, risking denial of service.

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/contract.rs#L192-L224 https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/capi/ecall_impl.rs#L198

Vulnerability details

MED: Manipulation of contract state or behavior LOC: https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/contract.rs#L192-L224

Description

The bare_call and contract_instantiate functions in contract.rs are vulnerable to code injection or parameter manipulation due to insufficient input validation and sanitization of user-controlled input data.

The bare_call function is responsible for executing a contract call with the provided input data, and the contract_instantiate function is responsible for instantiating a new contract with the provided code hash, input data, and salt. Both functions take user-controlled input data (input_data) and pass it directly to the underlying Contracts::bare_call and instantiate functions without proper validation or sanitization.

pub fn bare_call(
    address: AccountId,
    input_data: Vec<u8>,
    mode: ExecutionMode,
    tx_args: TransactionArguments,
) -> ContractExecResult {
    let result = contract_tx(origin.clone(), gas_limit, gas_free, move || {
        Contracts::bare_call(
            origin,
            address,
            transfer,
            gas_limit,
            storage_deposit_limit,
            input_data,
            pallet_contracts::DebugInfo::UnsafeDebug,
            pallet_contracts::CollectEvents::Skip,
            determinism,
        )
    });
    // ...
}

The issue lies in the lack of input validation and sanitization of the input_data parameter in both the bare_call and contract_instantiate functions. The user-controlled input data is passed directly to the underlying functions without any checks or sanitization, allowing potentially malicious or unexpected input to be processed.

The edge case responsible for the vulnerability are Contracts::bare_call(, input_data, and https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/capi/ecall_impl.rs#L198

Under normal circumstances, the bare_call and contract_instantiate functions are expected to execute contract calls or instantiate new contracts with valid and well-formed input data. The input data should conform to the expected format and constraints defined by the contract's ABI or the specific requirements of the contract instantiation process. But Due to the lack of input validation and sanitization, the bare_call and contract_instantiate functions allow arbitrary and potentially malicious input data to be passed to the underlying contract execution or instantiation functions. This can lead to code injection or parameter manipulation vulnerabilities, where an attacker can craft malicious input data to execute unintended or unauthorized actions within the contract.

Impact

Unauthorized access to sensitive contract functions or data hence denial of service or resource exhaustion attacks

Tools Used

Manual audit

Recommended Mitigation Steps

It is essential to implement proper input validation and sanitization mechanisms in the bare_call and contract_instantiate functions.

fn validate_input_data(input_data: &[u8]) -> Result<(), String> {
    // Perform length checks
    if input_data.len() > MAX_INPUT_LENGTH {
        return Err("Input data exceeds maximum length".to_string());
    }

    // Perform format and content checks
    // ...

    // Sanitize input data
    let sanitized_data = sanitize(input_data);

    // Additional validation and checks
    // ...

    Ok(())
}

pub fn bare_call(
    address: AccountId,
    input_data: Vec<u8>,
    mode: ExecutionMode,
    tx_args: TransactionArguments,
) -> ContractExecResult {
    match validate_input_data(&input_data) {
        Ok(_) => {
            // Input data is valid, proceed with the contract call
            let result = contract_tx(origin.clone(), gas_limit, gas_free, move || {
                Contracts::bare_call(
                    // ...
                    input_data,
                    // ...
                )
            });
            // ...
        }
        Err(err) => {
            // Input data is invalid, return an error
            log::error!("Invalid input data: {}", err);
            // Return appropriate error result
            // ...
        }
    }
}

Validate the length, format, and content of the input_data parameter to ensure it meets the expected criteria for the specific contract or instantiation process.

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory: Insufficient proof