scarf•15mo ago

rust ffi buffer overflow

hi, do i have to null-terminate array when using rust ffi? i'm getting buffer overflows.

#[no_mangle]
pub extern "C" fn play(path: *const u8) {
    let path = unsafe {
        assert!(!path.is_null());

        std::ffi::CStr::from_ptr(path as *const i8)
    };
    let path = path.to_str().unwrap();
    println!("path: {path}");
}

#[no_mangle]
pub extern "C" fn play(path: *const u8) {
    let path = unsafe {
        assert!(!path.is_null());

        std::ffi::CStr::from_ptr(path as *const i8)
    };
    let path = path.to_str().unwrap();
    println!("path: {path}");
}

const lib = "./audio/target/debug/libaudio.so"
const dylib = Deno.dlopen(lib, { "play": { parameters: ["buffer"], result: "void" } } as const)

const encoded = new TextEncoder().encode("path")

dylib.symbols.play(encoded)

const lib = "./audio/target/debug/libaudio.so"
const dylib = Deno.dlopen(lib, { "play": { parameters: ["buffer"], result: "void" } } as const)

const encoded = new TextEncoder().encode("path")

dylib.symbols.play(encoded)

$ deno run -A --unstable --allow-ffi audio.ts
thread '<unnamed>' panicked at src/lib.rs:29:30:
called `Result::unwrap()` on an `Err` value: Utf8Error { valid_up_to: 3, error_len: Some(1) }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5

$ deno run -A --unstable --allow-ffi audio.ts
thread '<unnamed>' panicked at src/lib.rs:29:30:
called `Result::unwrap()` on an `Err` value: Utf8Error { valid_up_to: 3, error_len: Some(1) }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5

3 Replies

Deno AI Helper•15mo ago

Based on the documentation, when calling FFI symbols with a buffer, the next argument must be the buffer’s length. However, in your code, you're not passing the buffer length to the play function. This could be the reason why you're experiencing buffer overflows. Here's how you can modify your Rust and Deno code to include the buffer length: Rust:

#[no_mangle]
pub extern "C" fn play(path: *const u8, len: usize) {
    let path = unsafe {
        assert!(!path.is_null());
        std::slice::from_raw_parts(path, len)
    };
    let path = std::str::from_utf8(path).unwrap();
    println!("path: {path}");
}

#[no_mangle]
pub extern "C" fn play(path: *const u8, len: usize) {
    let path = unsafe {
        assert!(!path.is_null());
        std::slice::from_raw_parts(path, len)
    };
    let path = std::str::from_utf8(path).unwrap();
    println!("path: {path}");
}

Deno:

const lib = "./audio/target/debug/libaudio.so"
const dylib = Deno.dlopen(lib, { "play": { parameters: ["buffer", "usize"], result: "void" } } as const)

const encoded = new TextEncoder().encode("path")

dylib.symbols.play(encoded, encoded.length)

const lib = "./audio/target/debug/libaudio.so"
const dylib = Deno.dlopen(lib, { "play": { parameters: ["buffer", "usize"], result: "void" } } as const)

const encoded = new TextEncoder().encode("path")

dylib.symbols.play(encoded, encoded.length)

In the Rust code, we're now accepting a length parameter and using std::slice::from_raw_parts to create a slice from the raw parts. In the Deno code, we're passing the length of the buffer as the second argument to the play function. Please try this modification and see if it resolves your issue. 🦕

Helpful Links: Deno 1.15 Release Notes Deno 1.17 Release Notes Deno 1.13 Release Notes Deno 1.13 Release Notes

AapoAlas•15mo ago

C strings are null terminated, so indeed you need to null terminate manually on the JS side. Alternatively, you can use the code suggested by the AI Helper and pass in the length of the buffer as well, creating a str which you can then turn into an owned String if need be.

scarfOP•15mo ago

thanks. i chose appending \0 to parameter before encoding.