Memory Corruption Vulnerabilities in Ocall's Default Allocator

[c4-bot-6]

Lines of code

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

Vulnerability details

The runtime executes arbitrary WebAssembly code provided by the user; therefore, any operation executed in the runtime should not be trusted. There are two types of calls, ecall and ocall, that may cross the module boundary. The ocall is particularly interesting as it allows the user to interact with the outside world voluntarily.

The ocall handler is registered inside __pink_runtime_init (phala-blockchain/crates/pink/runtime/src/capi/mod.rs) by ocall_impl::set_ocall_fn (phala-blockchain/crates/pink/runtime/src/capi/ocall_impl.rs). It is required by every runtime.

typedef struct
{
    // The outwards cross-call function of the runtime.
    cross_call_fn_t ocall;
    // Memory allocation functions.
    alloc_fn_t alloc;
    dealloc_fn_t dealloc;
} ocalls_t;

/// This is the entry point of the runtime. It will initialize the runtime and
/// fill the ecalls table.
///
/// # Safety
///
/// The caller should make sure the pointers are valid and non-null.
#[no_mangle]
pub unsafe extern "C" fn __pink_runtime_init(
    config: *const config_t,
    ecalls: *mut ecalls_t,
) -> ::core::ffi::c_int {
    let config = unsafe { &*config };
    if let Err(err) = ocall_impl::set_ocall_fn(config.ocalls) {
        log::error!("Failed to init runtime: {err}");
        return -1;
    }

The “allocator” feature is enabled by default. https://github.com/code-423n4/2024-03-phala-network/blob/main/phala-blockchain/crates/pink/runtime/Cargo.toml#L62

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(())
}

If a dedicated allocator is specified, the default allocator is overridden by the new value. In the runtime loader (phala-blockchain/crates/pink/loader/src/runtimes/v1.rs#L94-L95), they are conditionally set to the ones shipped with the runtime library.

unsafe extern "C" fn ocall_alloc(size: usize, align: usize) -> *mut u8 {
    std::alloc::alloc(std::alloc::Layout::from_size_align(size, align).unwrap())
}

unsafe extern "C" fn ocall_dealloc(ptr: *mut u8, size: usize, align: usize) {
    std::alloc::dealloc(
        ptr,
        std::alloc::Layout::from_size_align(size, align).unwrap(),
    )
}

Though the runtime is out of scope in this contest, it is basically the same as the default ones in phala-blockchain/crates/pink/runtime/src/capi/ocall_impl.rs#L86-L97

    #[global_allocator]
    static ALLOCATOR: PinkAllocator = PinkAllocator;

    unsafe impl GlobalAlloc for PinkAllocator {
        unsafe fn alloc(&self, layout: std::alloc::Layout) -> *mut u8 {
            ALLOC_FUNC(layout.size(), layout.align())
        }

        unsafe fn dealloc(&self, ptr: *mut u8, layout: std::alloc::Layout) {
            DEALLOC_FUNC(ptr, layout.size(), layout.align())
        }
    }

Leveraging the ocall to a system allocator is a hack to stabilize and simplify the metrics, as noted in the comment:

 //! # Rust memory allocator in dynamic runtimes
 //!
 //! By default, Rust std comes with a default allocator which uses the alloc
 //! APIs from libc. As a result, every dynamic library written in Rust will
 //! have its own allocator. This is not what we want because we have a common
 //! allocator in the main executable which have some metrices and statistics.
 //! If we use the default allocator, the statistics will be not accurate.
 //! So we make this allocator in the runtime and delegate the calls to the
 //! allocator in the main executable.

However, these unsafe functions serve just as a thin wrapper of the system's native allocator. Take Linux, for example, where the internal logic of memory management is handled by libc's malloc() and free() functions.

Since users have full control over the parameters used to invoke ocalls, they could potentially manipulate raw pointers in the external environment. For instance, they might free() memory through low-level ocalls.

Such actions could lead to further memory corruption and potentially compromise the entire runtime.

A single node might be vulnerable to hacking via off-chain queries, and similarly, the entire network could be at risk of being taken down by on-chain transactions.

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[141345]

no validation on set_ocall_fn()

[kvinwang]

The worker is running in TEE. All binary dependencies, including libc, are checked before launch. It is impossible for an attacker to replace the allocator implementation.

[c4-sponsor]

kvinwang (sponsor) disputed

[OpenCoreCH]

Interesting attack idea. @kvinwang Even in TEE and with a valid libc, I'm wondering if it is possible for an attacker to provide a pointer to memory they did not allocate when deallocating, which will lead to a free call on this pointer and might lead to memory corruption of the host? I could not find anything in the codebase that prevents this, but it is possible that I am missing something.

[kvinwang]

I'm wondering if it is possible for an attacker to provide a pointer to memory they did not allocate when deallocating, which will lead to a free call on this pointer and might lead to memory corruption of the host?

Contracts are WebAssembly programs running inside a virtual machine. They cannot provide a pointer to a free call in the host side. If they can, then the system is already compromised, and there is no longer a need to manipulate free/malloc operations.

---- Update ---- To be clear, the sandbox boundary is not the ecalls/ocalls boundary between the pink runtime and worker but the wasm vm implemented inside pallet-contracts.

[OpenCoreCH]

I'm wondering if it is possible for an attacker to provide a pointer to memory they did not allocate when deallocating, which will lead to a free call on this pointer and might lead to memory corruption of the host?

Contracts are WebAssembly programs running inside a virtual machine. They cannot provide a pointer to a free call in the host side. If they can, then the system is already compromised, and there is no longer a need to manipulate free/malloc operations.

---- Update ---- To be clear, the sandbox boundary is not the ecalls/ocalls boundary between the pink runtime and worker but the wasm vm implemented inside pallet-contracts.

Ok thanks that makes sense, so the memory isolation is provided by WASM VM, which does not allow any accesses beyond the memory that is allocated to the VM.

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory: Invalid