fix(security): 1.35.6 hardening stack (14-vector audit remediation)

[andypost]

Summary

The consolidated 1.35.6 security hardening — the code remediations for a 14-vector security audit (catalogued in andypost/unit#10, security-audit.md), organized as one commit per area (the PR-A..PR-I stack), plus three follow-up fixes from automated review. CHANGES is updated; there is no behavioral change beyond the documented hardening.

Remediations (PR-A..PR-I)

Commit	Area	Audit vectors
0e194531	fix(websocket) — frame-bound checks in libunit + protocol	PR-A: V10 hsize OOB read, V12 frame-size decrement / 64-bit length, V8 ASGI overflow, V9 Java
f1ff3769	fix(port) — bounds-check untrusted shmem offsets	PR-B: V5 chunk_id bounds, V10 sptr offset
03e6b0ee	fix(isolation) — control-socket / mount / cgroup boundaries	PR-C: V4 SO_PEERCRED, V6 cgroup & mount-dest TOCTOU
84c340fa	fix(tls) — cert/key pair + wildcard SAN matcher	PR-D: V3 SSL_CTX_check_private_key, wildcard SAN OOB
73e4190b	fix(fd) — FD/CLOEXEC lifetime (accept/pipe/compression)	PR-E: V14 CLOEXEC on accepted/piped fds, V11 compression mmap fd
f2596110	fix(http) — proxy Content-Length + URI/string helpers	PR-F: V1 proxy CL underflow (smuggling), V13 nxt_is_complex_uri_encoded off-by-one
887e7949	fix(sapi) — bounds-check app-supplied arguments	PR-G: V7 PHP header skip, V8 WSGI, V9 Java/WASM bounds
b59c55b1	fix(controller) — JSON parser caps + request state	PR-H: V4 JSON depth/element caps, V7 PHP async, V8 Ruby rack.input bleed
499de4b2	fix(routing) — CIDR / regex / port-range matcher edges	PR-I: V2 ACL-bypass edges
6b8e72ce	docs(changes) — record 1.35.6 hardening fixes	—

Follow-up fixes (from automated review)

Commit	Fixes a defect in	Note
bb58aed6	f1ff3769 (V10)	nxt_unit_sptr_in_buf() buf_size - sizeof() underflowed for sub-struct buffers, bypassing the OOB guard. Security.
6dde1b4d	03e6b0ee (V6)	rootfs prefix matched siblings (<rootfs>-helper) → spurious ENOENT / startup abort. Require a / separator.
ca2fa45b	73e4190b (V14)	pipe2() probed compile-only → ENOSYS broke pipe creation on older kernels. Fall back to pipe()+FD_CLOEXEC.

Notes / scope

• Threat model excludes pure DoS (fd-exhaustion shapes, JSON-cap self-DoS under already-authenticated CONTROL); those are hardened opportunistically, not the focus. See security-audit.md for the per-vector exploitability analysis.
• The flaky go-1.25 test_go_isolation_rootfs_automount_tmpfs[_regression] is the rootfs automount reload race fixed separately in fix(isolation): close rootfs automount reload race (#83) #84 (approved). Once fix(isolation): close rootfs automount reload race (#83) #84 lands on pre-1.35.6, this branch rebases and that job goes green; the hardening here is independent of it.
• Builds clean (unitd + language modules link; verified locally for the daemon and the PHP module). Full build-test matrix runs on this PR.
• Targets pre-1.35.6.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive set of security and robustness bugfixes across NGINX Unit, including hardened JNI bindings, stricter WebSocket frame validation, bounds-checked WASM guest offsets, and improved isolation boundaries via openat2 and peer credential checks. The code review successfully identified several critical issues in these changes: potential JVM crashes from missing NULL checks on JNI array parameters, a resource leak when truncating HTTP/1.1 proxy buffer chains, out-of-bounds read hazards in WebSocket frame header parsing, and a potential uninitialized stack memory read during getsockopt peer credential checks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The out array parameter is not checked for NULL before calling GetArrayLength. If a Java caller passes null, this will cause a JVM crash. A defensive NULL check should be added first.

    if (out == NULL) {
        nxt_java_throw_IllegalStateException(env,
            "InputStream.readLine: output array is null");
        return -1;
    }

    array_len = (*env)->GetArrayLength(env, out);
    if (off < 0 || len < 0 || off > array_len || len > array_len - off) {
        nxt_java_throw_IllegalStateException(env,
            "InputStream.readLine: off/len out of bounds");
        return -1;
    }

[gemini-code-assist]

The arr parameter is not checked for NULL before calling GetArrayLength. If a Java caller passes null, this will cause a JVM crash. A defensive NULL check should be added first.

    if (arr == NULL) {
        nxt_java_throw_IllegalStateException(env,
            "sendWsFrame: array is null");
        return;
    }

    cap = (*env)->GetArrayLength(env, arr);
    if (pos < 0 || len < 0
        || pos > cap
        || len > cap - pos)
    {
        nxt_java_throw_IllegalStateException(env,
            "sendWsFrame: pos/len out of array length");
        return;
    }

[gemini-code-assist]

Truncating the buffer chain by setting b->next = NULL without freeing the remaining buffers in the chain will leak resources (such as shared memory chunks if they are port mmap buffers). Free the rest of the chain before truncating.

                /* Trim this buf to fit, drop everything after it. */
                b->mem.free = b->mem.pos
                              + (size_t) h1p->remainder - trimmed;
                nxt_buf_t *next = b->next;
                b->next = NULL;
                while (next != NULL) {
                    nxt_buf_t *temp = next->next;
                    nxt_buf_free(task, next);
                    next = temp;
                }
                break;

[gemini-code-assist]

The function nxt_websocket_frame_header_size reads the first 2 bytes of the header to determine its size. If the buffer has fewer than 2 bytes, calling it will result in an out-of-bounds read. Defensively check that the buffer has at least 2 bytes first.

        if (nxt_slow_path((size_t) (b->buf.end - b->buf.start) < 2)) {
            nxt_unit_warn(ctx, "#%"PRIu32": truncated websocket frame: "
                          "buf size %zu < 2",
                          req_impl->stream,
                          (size_t) (b->buf.end - b->buf.start));

            nxt_unit_websocket_frame_release(&ws_impl->ws);

            return NXT_UNIT_ERROR;
        }

        hsize = nxt_websocket_frame_header_size(ws_impl->ws.header);

        /*
         * Reject truncated frames before reading the extended length /
         * mask fields or advancing buf.free past buf.end.  A 2-byte
         * frame whose header advertises a 14-byte extended length would
         * otherwise OOB-read b->buf.start + hsize - 4 (mask) and the
         * 8-byte extended length, and break the buffer invariant.
         */
        if (nxt_slow_path((size_t) (b->buf.end - b->buf.start) < hsize)) {

[gemini-code-assist]

The function nxt_websocket_frame_header_size reads the first 2 bytes of the header to determine its size. If size < 2, calling it will result in an out-of-bounds read. Defensively check that size >= 2 first.

    if (nxt_slow_path(size < 2)) {
        nxt_unit_free(ws->req->ctx, b);
        return NXT_UNIT_ERROR;
    }

    hsize = nxt_websocket_frame_header_size(b);

    /* Same OOB-read hazard as nxt_unit_process_websocket(). */
    if (nxt_slow_path(hsize > size)) {

[gemini-code-assist]

The getsockopt call can succeed but return a smaller len than sizeof(struct ucred). To prevent reading uninitialized stack memory, defensively verify that len is at least sizeof(struct ucred).

        if (getsockopt(c->socket.fd, SOL_SOCKET, SO_PEERCRED, &cred, &len)
            != 0 || len < sizeof(cred))
        {

[TuzelKO]

@claude review once

[claude]

Claude finished @TuzelKO's task in 6m 33s —— View job

---

Code Review — PR #85: 1.35.6 Hardening Stack

• Gather context and understand the PR
• Read the diff against origin/pre-1.35.6
• Review key changed files
• Verify Gemini Code Assist findings
• Post complete review

---

This is a substantial security hardening PR. The broad shape is sound, and most fixes are correct. The Gemini Code Assist findings I verified below are all real bugs that survived this PR's review cycle and should be fixed before merge.

---

Critical / High — Must Fix

1. src/java/nxt_jni_InputStream.c:107 — JVM crash on null out array (confirms Gemini)

GetArrayLength(env, out) is called unconditionally. Per JNI spec, calling any JNI function (including GetArrayLength) with a null jobject invokes undefined behaviour and typically crashes the JVM:

// Current (line 107) — crashes if `out` is null
array_len = (*env)->GetArrayLength(env, out);

The fix is a NULL guard before the length call:

if (out == NULL) {
    nxt_java_throw_IllegalStateException(env,
        "InputStream.readLine: output array is null");
    return -1;
}
array_len = (*env)->GetArrayLength(env, out);

Fix this →

---

2. src/java/nxt_jni_Request.c:768 — JVM crash on null arr array (confirms Gemini)

Same pattern: GetArrayLength(env, arr) at line 768 with no prior null check:

// Current (line 768) — crashes if `arr` is null
cap = (*env)->GetArrayLength(env, arr);

Fix:

if (arr == NULL) {
    nxt_java_throw_IllegalStateException(env,
        "sendWsFrame: array is null");
    return;
}
cap = (*env)->GetArrayLength(env, arr);

Fix this →

---

3. src/nxt_unit.c:1808 — OOB read before the truncation guard (confirms Gemini)

nxt_websocket_frame_header_size() reads the first 2 bytes of the buffer to examine payload_len and mask bits. The truncation guard is at line 1817, which is after those 2 bytes are already read:

// Line 1808 — reads 2 bytes; buf might be 0 or 1 byte
hsize = nxt_websocket_frame_header_size(ws_impl->ws.header);

// Line 1817 — too late; the OOB read already happened
if (nxt_slow_path((size_t) (b->buf.end - b->buf.start) < hsize)) {

A compromised application module can craft an mmap message with size < 2 to trigger the OOB read. The fix is a >= 2 pre-check:

if (nxt_slow_path((size_t)(b->buf.end - b->buf.start) < 2)) {
    nxt_unit_warn(ctx, "#%" PRIu32 ": truncated websocket frame header",
                  req_impl->stream);
    nxt_unit_websocket_frame_release(&ws_impl->ws);
    return NXT_UNIT_ERROR;
}
hsize = nxt_websocket_frame_header_size(ws_impl->ws.header);
if (nxt_slow_path((size_t)(b->buf.end - b->buf.start) < hsize)) { ... }

Fix this →

---

4. src/nxt_unit.c:3603 — Same OOB pattern in nxt_unit_websocket_retain() (confirms Gemini)

// Line 3594
size = ws_impl->buf->buf.end - ws_impl->buf->buf.start;
// ...
// Line 3603 — reads 2 bytes; size could be 0 or 1
hsize = nxt_websocket_frame_header_size(b);
// Line 3606 — after-the-fact
if (nxt_slow_path(hsize > size)) { ... }

Insert if (size < 2) { nxt_unit_free(...); return NXT_UNIT_ERROR; } before line 3603.

Fix this →

---

Medium — Should Fix

5. src/nxt_controller.c:734 — Partial getsockopt result leaves cred uninitialized (confirms Gemini)

POSIX allows getsockopt to succeed while returning fewer bytes than requested (it writes len with the actual size filled). The current check only tests the return code:

// Current — `cred` may be partially filled
if (getsockopt(c->socket.fd, SOL_SOCKET, SO_PEERCRED, &cred, &len) != 0)

The fix adds a size guard:

if (getsockopt(c->socket.fd, SOL_SOCKET, SO_PEERCRED, &cred, &len) != 0
    || len < (socklen_t) sizeof(cred))

Fix this →

---

6. src/nxt_h1proto.c:2981 — Chain tail dropped without releasing buffers (confirms Gemini)

// Line 2981 — orphans everything after b
b->next = NULL;
break;

The nodes dropped from the chain hold memory-pool or potentially mmap-backed allocations. On this code path they are HTTP/1.1 proxy read buffers (not port-mmap chunks), so the memory is reclaimed when the pool is freed, but the pattern is fragile if the buffer type ever changes. Freeing the tail explicitly is safer:

{
    nxt_buf_t *nxt = b->next;
    b->next = NULL;
    while (nxt != NULL) {
        nxt_buf_t *tmp = nxt->next;
        /* nxt_buf_free(task, nxt); if reference-counted */
        nxt = tmp;
    }
}

---

Low / Informational

7. Pre-existing typo: hxt_h1p_send_ws_error (src/nxt_h1proto_websocket.c:33)

The PR correctly uses the existing hxt_ prefix (it was there before this PR), but the function name should arguably be nxt_h1p_send_ws_error. This is a cosmetic issue inherited from the codebase; the new call added by the PR (line 281) is consistent with existing callers.

---

What the PR Gets Right

The following changes are well-implemented:

• nxt_port_memory.c — The nxt_unit_sptr_in_buf() fix correctly documents and fixes the buf_size - sizeof() underflow that would have bypassed the OOB guard. The comment is clear.
• nxt_string.c:719 — The < 2 → < 3 fix for nxt_is_complex_uri_encoded correctly accounts for the 3-byte %XX sequence.
• nxt_openssl.c — SSL_CTX_check_private_key() addition and the length > 0 guard on wildcard SAN are both correct.
• nxt_wasm.c — The bounds checks on offset, nfields, and each (name_off, name_len, val_off, val_len) tuple are thorough; the name_len > UINT8_MAX rejection is a good catch for the silent truncation.
• nxt_h1proto_websocket.c — The RFC 6455 §5.2 MSB check (payload_len == 127 && payload_len_[0] & 0x80) is correctly scoped to the 64-bit extended length only.
• nxt_ruby_stream_io.c — The nxt_ruby_io_bind_t + req_seq guard is the right fix for rack.input cross-request bleed.
• nxt_conf.c — JSON depth/element caps are documented and the depth counter is correctly reset on each entry to nxt_conf_json_parse().
• nxt_isolation.c — openat2 RESOLVE_BENEATH check is gated on kernel availability with a clear fallback warning; the dst[rootfs_len] == '/' path-separator fix prevents sibling-path false matches.
• nxt_php_sapi.c — The isspace((unsigned char) *value) cast, the bounds check value < h->header + h->header_len, and the zend_clear_exception() in async mode are all correct.

---

Summary: 4 confirmed security bugs (2 JVM-crash vectors, 2 WebSocket OOB reads) and 1 partial-read/uninitialized-memory issue should be addressed before merge. The broad structure of the PR is solid.