napi-rs icon indicating copy to clipboard operation
napi-rs copied to clipboard

Creating an Array Buffer using native code can lead to crashes/unsound code

Open Janrupf opened this issue 1 year ago • 3 comments

Creating buffers using Env::create_arraybuffer* can lead to unsound Rust code without using unsafe.

Take the following Rust code for example (lib.rs):

use napi::{Env, JsArrayBuffer, JsBuffer};

#[napi_derive::napi]
pub fn create_buffer(env: Env) -> napi::Result<JsArrayBuffer> {
    let v = env.create_arraybuffer(16)?;

    Ok(v.into_raw())
}

#[napi_derive::napi]
pub fn consume_buffer(buffer: JsBuffer) -> napi::Result<()> {
    let _ = buffer.into_value()?; // <-- This will trigger an assertion (leading to a SIGABRT)

    Ok(())
}

Attempting to use it in node CLI:

> const m = require("/path/to/module-from-above.node");
undefined
> const buffer = m.createBuffer();
undefined
> buffer
ArrayBuffer {
  [Uint8Contents]: <00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00>,
  byteLength: 16
}
> m.consumeBuffer(buffer);

  #  node[44332]: char* node::Buffer::Data(v8::Local<v8::Value>) at ../src/node_buffer.cc:248
  #  Assertion failed: val->IsArrayBufferView()

----- Native stack trace -----

 1: 0xdf33d7 node::Assert(node::AssertionInfo const&) [node]
 2: 0xdc0c0a node::Buffer::Data(v8::Local<v8::Value>) [node]
 3: 0xdb3a1e napi_get_buffer_info [node]
 4: 0x7a5b8e8b9e47  [/path/to/module-from-above.node]
 5: 0x7a5b8e8a7c2b  [/path/to/module-from-above.node]
 6: 0x7a5b8e8a7640  [/path/to/module-from-above.node]
 7: 0x7a5b8e8a8d7d  [/path/to/module-from-above.node]
 8: 0x7a5b8e8a75ba  [/path/to/module-from-above.node]
 9: 0x7a5b8e8a8b5d  [/path/to/module-from-above.node]
10: 0x7a5b8e8a7d0a  [/path/to/module-from-above.node]
11: 0xd8dd09  [node]
12: 0x7a5b8720eadd 

----- JavaScript stack trace -----

1: REPL39:1:3
2: runInThisContext (node:vm:121:12)
3: defaultEval (node:repl:599:22)
4: bound (node:domain:432:15)
5: runBound (node:domain:443:12)
6: onLine (node:repl:929:10)
7: emit (node:events:531:35)
8: emit (node:domain:488:12)
9: [_onLine] (node:internal/readline/interface:416:12)
10: [_line] (node:internal/readline/interface:887:18)


[1]    44332 IOT instruction (core dumped)  node -i

Interestingly enough, running buffer.is_buffer() returns false, which explains the assertion error.

For context:

let v = env.create_arraybuffer(16)?;
let v = v.into_raw()?;

v.is_buffer()? // <- false

Looking at the implementation of ValidateNapiValue for JsBuffer, this should be caught but is not.

Janrupf avatar Jan 25 '24 20:01 Janrupf

Looking at the implementation of ValidateNapiValue for JsBuffer, this should be caught but is not.

The validate method is only called under the #[napi(strict)] mode

Brooooooklyn avatar Jan 26 '24 04:01 Brooooooklyn

I believe this is a bug in Node.js, Node.js should return a napi_invalid_arg error instead of crashing.

Brooooooklyn avatar Jan 26 '24 05:01 Brooooooklyn

Filed a bug report: https://github.com/nodejs/node/issues/51570

Janrupf avatar Jan 26 '24 16:01 Janrupf