[Security][Medium] path.posix.basename UB on empty string, NULL byte in fs, detached ArrayBuffer in zlib

[JkarVN]

Summary

Three minor security issues across path, fs, and zlib modules.

---

Bug 1: Undefined Behavior — path.posix.basename("") crashes

File: path.cpp:513–515

void Path::basenamePosix(const v8::FunctionCallbackInfo<v8::Value>& args) {
    ...
    std::string p = *val;
    if (p.back() == '/')    // ← UB on empty std::string!
        p.pop_back();

Calling path.posix.basename("") invokes std::string::back() on an empty string, which is undefined behavior (C++ standard §24.4.5.2). In practice this reads out-of-bounds heap memory, potentially leaking data or causing a crash (DoS).

PoC

const path = require('path').posix;
path.basename("");  // UB → possible crash or info leak

Fix

if (!p.empty() && p.back() == '/') p.pop_back();

Note: The host Path::basename() (line 293) uses p.filename() which handles empty paths correctly, so only the Posix variant is affected. Should audit all Posix functions for similar empty-string UB.

---

Bug 2: NULL byte in fs path — currently safe but fragile

File: fs.cpp (all path-accepting functions)

v8::String::Utf8Value returns a NUL-terminated C string, so isPathSafe(*path_val) and std::ifstream(*path_val) both see the truncated string. This means a JS string like "file\x00.txt" is effectively "file". No injection is possible today.

However, this safety is accidental and fragile:

• If any future code uses Utf8Value::length() to construct a std::string (which preserves embedded NULs), the NUL bypass would reappear.
• Attackers commonly use NUL bytes to bypass path validation in C/C++ applications.

Fix (defensive)

if (memchr(*path_val, '\0', strlen(*path_val))) {
    // V8 shouldn't produce embedded NUL, but reject if it does
    isolate->ThrowException(...);
    return;
}

---

Bug 3: Detached ArrayBuffer in zlib dictionary

File: zlib.cpp:119, 132, 180, 217

uint8_t* p_data = static_cast<uint8_t*>(view->Buffer()->GetBackingStore()->Data());
dictionary.assign(p_data, p_data + view->ByteLength());

No check for view->Buffer()->IsDetached(). If a user detaches the ArrayBuffer between the V8 call and the async execution (in async functions), the BackingStore data pointer becomes stale, causing a use-after-free — reading freed memory.

PoC (theoretical, async only)

const zlib = require('zlib');
const buf = new Uint8Array(16);
const dict = new Uint8Array(16);
zlib.inflate(buf, { dictionary: dict.buffer }, (err, result) => {});
// Between enqueue and execution:
dict.buffer.detach();  // stale pointer in thread pool

Fix

if (view->Buffer()->IsDetached()) {
    isolate->ThrowException(v8::Exception::TypeError(
        v8::String::NewFromUtf8Literal(isolate, "ArrayBuffer is detached")));
    return;
}

Also: parseZlibOptions parses the dictionary key twice (lines 116 and 129), with the second parse overwriting the first. The duplicate should be removed.