c4-bot-2 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/capi/ocall_impl.rs#L18-L34

Vulnerability details

MED: set_ocall_fn function is to set the OCALL, ALLOC_FUNC, and DEALLOC_FUNC function pointers based on the provided ocalls parameter. LOC: Panic in set_ocall_fn crates/pink/runtime/src/capi/ocall_impl.rs:18-33

Description

The set_ocall_fn function uses unsafe blocks to set the OCALL, ALLOC_FUNC, and DEALLOC_FUNC function pointers. If these pointers are not properly validated or if they are set to invalid or malicious functions, it could lead to undefined behavior or security vulnerabilities.

pub(super) fn set_ocall_fn(ocalls: ocalls_t) -> Result<(), &'static str> {
    let Some(ocall) = ocalls.ocall else {
        return Err("No ocall function provided");
    };
    unsafe {
        OCALL = ocall;
        #[cfg(feature = "allocator")]
        if let Some(alloc) = ocalls.alloc {
            allocator::ALLOC_FUNC = alloc;
        }
        #[cfg(feature = "allocator")]
        if let Some(dealloc) = ocalls.dealloc {
            allocator::DEALLOC_FUNC = dealloc;
        }
    }
    Ok(())
}

The set_ocall_fn is to set the OCALL, ALLOC_FUNC, and DEALLOC_FUNC function pointers to valid and safe functions provided through the ocalls parameter. These pointers should point to trusted and well-defined functions that perform the intended operations safely.

The issue is lack of proper validation of the function pointers, an attacker can potentially provide malicious function pointers that point to arbitrary code or invalid memory locations. When these malicious function pointers are called, it can lead to undefined behavior, crashes, or the execution of arbitrary code.

The use of unsafe blocks to set the function pointers without proper validation the code assumes that the provided function pointers are valid and safe to call. However, if an attacker can control the ocalls parameter and provide malicious function pointers, it could lead to undefined behavior or security vulnerabilities.

Impact

This can lead to a complete compromise of the system, allowing the attacker to perform unauthorized actions, access sensitive data, or disrupt the normal functioning of the runtime.

Tools Used

Manual audit

Recommended Mitigation Steps

Ensure that the ocalls parameter is properly validated before setting the function pointers. Consider implementing additional safety checks and error handling to prevent the setting of invalid or malicious function pointers.

fn is_valid_ocall_fn(ocall: extern "C" fn(...) -> ...) -> bool {
    // Check if the ocall function is in the allowlist
    // Return true if it's a valid and trusted function
    // ...
}

fn is_valid_alloc_fn(alloc: extern "C" fn(...) -> *mut u8) -> bool {
    // Check if the alloc function is in the allowlist
    // Return true if it's a valid and trusted function
    // ...
}

fn is_valid_dealloc_fn(dealloc: extern "C" fn(...)) -> bool {
    // Check if the dealloc function is in the allowlist
    // Return true if it's a valid and trusted function
    // ...
}

pub(super) fn set_ocall_fn(ocalls: ocalls_t) -> Result<(), &'static str> {
    let Some(ocall) = ocalls.ocall else {
        return Err("No ocall function provided");
    };
    if !is_valid_ocall_fn(ocall) {
        return Err("Invalid ocall function");
    }
    unsafe {
        OCALL = ocall;
    }
    #[cfg(feature = "allocator")]
    if let Some(alloc) = ocalls.alloc {
        if !is_valid_alloc_fn(alloc) {
            return Err("Invalid alloc function");
        }
        unsafe {
            allocator::ALLOC_FUNC = alloc;
        }
    }
    #[cfg(feature = "allocator")]
    if let Some(dealloc) = ocalls.dealloc {
        if !is_valid_dealloc_fn(dealloc) {
            return Err("Invalid dealloc function");
        }
        unsafe {
            allocator::DEALLOC_FUNC = dealloc;
        }
    }
    Ok(())
}

In this, separate validation functions (is_valid_ocall_fn, is_valid_alloc_fn, is_valid_dealloc_fn) are introduced to check if the provided function pointers are in the allowlist of trusted functions. The set_ocall_fn function is modified to perform these validations before setting the function pointers. If any of the validations fail, an error is returned, preventing the setting of invalid function pointers.

Assessed type

Access Control

c4-pre-sort commented 3 months ago

141345 marked the issue as duplicate of #82

c4-judge commented 3 months ago

OpenCoreCH marked the issue as not a duplicate

OpenCoreCH commented 3 months ago

This is known and the responsibility of the pink runtime init caller:

/// # Safety
///
/// The caller should make sure the pointers are valid and non-null.