nfssvr.sys — XDR error-path redundant null-terminator write removed
KB5073723
1. Overview
- Unpatched Binary:
nfssvr_unpatched.sys - Patched Binary:
nfssvr_patched.sys - Overall Similarity Score: 0.9927
- Diff Statistics: 1992 matched functions, 3 changed functions, 1989 identical functions, 0 unmatched functions in either direction.
- Verdict: The patch retargets the XDR-error branch in the NFS server's string/opaque decoder so that a null-terminator write and a no-op slow-path call are skipped when the XDR stream is already in an error state. In every caller the write target is inside the destination allocation, so no out-of-bounds access occurs in either build. This is a correctness / defense-in-depth cleanup with no demonstrable security impact.
2. Change Summary
- Severity: None (informational).
- Vulnerability Class: None. No memory-safety defect is present in either build.
- Affected Functions:
XdrDecodeString,Nfs4SrvCppDecodeCompoundHeaderAndInitializeArgs(inlined equivalent), andNlmGetClientAddressAndConnection(unrelated recompilation churn).
What actually changed
XdrDecodeString decodes a fixed-length opaque/string of length bytes into a caller-supplied buffer and then writes a terminator byte at buffer[length]. In the unpatched build, when the XDR context is already in an error state on entry (the signed dword at context+0x108 is negative), control still falls through to the terminator write (and to a call of XdrDecodeOpaqueSlow, which is a no-op on an error stream). The patched build retargets the error branch straight to the function epilogue, so on the error path neither the slow-path call nor the terminator write executes. The same inlined logic in the NFSv4 COMPOUND header decoder was changed the same way.
Why this is not a security fix
In every caller the destination buffer is sized length + 1 and the same length value is used both for the allocation and as the write index, so buffer[length] is the last valid byte of the allocation, not an out-of-bounds location:
XdrDecodeStringpool callers (AddMountEntry,RemoveMountEntry): allocateExAllocatePoolWithTag(PagedPool, length + 1, 'Nfse')and then callXdrDecodeString(ctx, length, buffer). The write atbuffer[length]is in bounds.XdrDecodeStringscratch-buffer callers: obtain alength + 1scratch buffer viaNfs4SrvCpGetScratchBufferand only callXdrDecodeStringwhencontext+0x108 >= 0, so the changed error branch is never reached from these paths.- Inlined COMPOUND tag decode: the tag length is rejected when it exceeds
0x400; the COMPOUND args buffer is allocated with tag'ARGS'and size2 * (184 * op_count + ((tag_length + 16) & ~0xF)) + 3920, and the tag string destination isbuffer + 3920. The write at(buffer + 3920)[tag_length]withtag_length <= 0x400is inside the allocation.
Because the destination is always the caller's own length + 1 allocation (which each caller discards immediately after when the context is in error), the removed write has no out-of-bounds, information-disclosure, or use-after-free effect. It is a redundant in-bounds store that the patch elides on the error path.
3. Decompiled Diff
XdrDecodeString @ 0x1C001DFEC
// UNPATCHED
v4 = a2; // length
if ( *(int *)(a1 + 264) < 0 ) // XDR context already in error state
goto LABEL_17; // -> XdrDecodeOpaqueSlow, then falls into the write
...
if ( v7 < (unsigned int)v4 )
LABEL_17:
result = XdrDecodeOpaqueSlow(a1, v4, a3); // no-op on an error stream
else
memmove(a3, v11, v4); // ... advance stream position ...
*((_BYTE *)a3 + v4) = 0; // terminator write (also runs on the error path)
// PATCHED
v4 = a2;
if ( *(int *)(a1 + 264) >= 0 ) // only when NOT in an error state
{
...
if ( v7 < (unsigned int)v4 )
XdrDecodeOpaqueSlow(a1, v4, a3);
else
memmove(a3, v11, v4); // ... advance stream position ...
*((_BYTE *)a3 + v4) = 0; // terminator write only on the success path
}
// error path: return immediately
4. Assembly Analysis
XdrDecodeString (Unpatched)
0x1C001DFFB cmp dword ptr [rcx+108h], 0 ; check XDR error status
0x1C001E002 mov rsi, r8 ; rsi = arg3 (dest buffer)
0x1C001E005 mov edi, edx ; edi = arg2 (string length)
0x1C001E007 mov rbx, rcx ; rbx = arg1 (XDR context)
0x1C001E00A jl loc_1C001E091 ; error -> slow-path block, falls into the write
...
0x1C001E091 mov r8, rsi
0x1C001E094 mov edx, edi
0x1C001E096 mov rcx, rbx
0x1C001E099 call XdrDecodeOpaqueSlow ; no-op on an error stream
0x1C001E09E mov byte ptr [rdi+rsi], 0 ; terminator write at dest[length]
0x1C001E0A2 mov rbx, [rsp+28h+arg_0] ; epilogue
0x1C001E0B1 retn
XdrDecodeString (Patched)
The only instruction that changed is the error branch target: 0x1C001E00A jl loc_1C001E091 becomes 0x1C001E00A jl loc_1C001E0A2, jumping straight to the epilogue and skipping both the XdrDecodeOpaqueSlow call and the terminator write on the error path. The rest of the function is byte-identical.
Nfs4SrvCppDecodeCompoundHeaderAndInitializeArgs @ 0x1C00084E8 (Unpatched inlined variant)
0x1C00089F7 mov rdi, [rsi+38h] ; rdi = tag buffer (ARGS allocation + 3920)
0x1C00089FB test eax, eax ; eax = saved XDR status
0x1C00089FD js loc_1C0008A83 ; error -> slow-path block, falls into the write
...
0x1C0008A83 mov r8, rdi
0x1C0008A86 mov edx, r14d
0x1C0008A89 mov rcx, rbx
0x1C0008A8C call XdrDecodeOpaqueSlow ; no-op on an error stream
0x1C0008A91 mov [r12+rdi], r13b ; terminator write at tag_buf[tag_length]
0x1C0008A95 mov rax, [rbx+48h] ; advance XDR pointer past the padding
0x1C0008A99 add qword ptr [rax+40h], 4
Nfs4SrvCppDecodeCompoundHeaderAndInitializeArgs (Patched inlined variant)
The error branch is retargeted: 0x1C00089FD js loc_1C0008A83 becomes 0x1C00089FD js loc_1C0008A95, jumping past both the XdrDecodeOpaqueSlow call and the terminator write at 0x1C0008A91 and continuing at the padding-advance code. The remaining differences in this function are register-allocation renames (r12 <-> r15) with no behavioral effect.
5. Reachability
The XDR string/opaque decoder is reached while parsing NFS requests (NFSv4 COMPOUND tag decoding in the inlined variant, and MOUNT path decoding via AddMountEntry / RemoveMountEntry for the standalone function). A malformed or truncated request can place the XDR context in an error state, which is the path the patch touches. However, because the destination is always the caller's length + 1 allocation (or the error path is not reached at all in the scratch-buffer callers), reaching this code produces at most a single in-bounds byte store into a buffer the caller then discards. There is no out-of-bounds write, no crash, and no attacker-controlled write offset outside an allocation.
6. Exploit Primitive & Development Notes
No exploit primitive is provided. The elided write in the unpatched build targets destination_buffer[length], which is the final byte of a length + 1 allocation in every caller, so it does not corrupt pool metadata or adjacent allocations and does not yield a memory-corruption primitive.
7. Debugger Notes
Not applicable: there is no reachable out-of-bounds condition to observe. To confirm the change itself, compare the error-branch target in XdrDecodeString at 0x1C001E00A (jl 0x1C001E091 unpatched vs jl 0x1C001E0A2 patched) and the inlined branch in Nfs4SrvCppDecodeCompoundHeaderAndInitializeArgs at 0x1C00089FD (js 0x1C0008A83 unpatched vs js 0x1C0008A95 patched).
8. Changed Functions — Full Triage
XdrDecodeString(@ 0x1C001DFEC, Similarity: 0.9395)- Change Type: Behavioral, non-security.
- Note: Error-branch target changed from the slow-path block (
0x1C001E091) to the epilogue (0x1C001E0A2), so theXdrDecodeOpaqueSlowno-op call and thedest[length]terminator write are skipped when the XDR context is in an error state. The terminator write is in bounds in all callers (length + 1allocation), so this is a redundant-write removal, not a bounds fix. Nfs4SrvCppDecodeCompoundHeaderAndInitializeArgs(@ 0x1C00084E8, Similarity: 0.9828)- Change Type: Behavioral, non-security.
- Note: The inlined COMPOUND tag decode receives the same error-branch retarget (
jsat0x1C00089FDfrom0x1C0008A83to0x1C0008A95), plusr12<->r15register renames. The tag destination isARGS_buffer + 3920and the tag length is capped at0x400, so the write is in bounds. NlmGetClientAddressAndConnection(@ 0x1C00CB958, Similarity: 0.9648)- Change Type: Compiler / Cosmetic.
- Note: Stack frame grew by
0x20bytes and register allocation shifted (includingrsi->r14), producing additional register spills and inverted branch conditions with reordered blocks. The call set (37 calls, identical targets) and comparison count (93) are unchanged. No functional or security impact. Its frame growth relocates the addresses of all subsequent functions in the image; those relocations are not code changes.
9. Unmatched Functions
- Added: None.
- Removed: None.
10. Confidence & Caveats
- Confidence: High. An independent instruction-stream comparison (with addresses and relocation labels normalized) confirms that only the three functions above changed; every other difference across the image is an address relocation caused by the
NlmGetClientAddressAndConnectionframe-size growth. - Basis: The destination allocation sizes were read directly from the callers:
ExAllocatePoolWithTag(PagedPool, length + 1, 'Nfse')inAddMountEntry/RemoveMountEntry,Nfs4SrvCpGetScratchBuffer(..., length + 1, ...)in the scratch-buffer callers, andExAllocatePoolWithTag(..., 2 * (184 * op_count + ((tag_length + 16) & ~0xF)) + 3920, 'ARGS')with the tag string placed atbuffer + 3920in the COMPOUND header decoder. In all cases the terminator write index is within the allocation, so the removed write is not an out-of-bounds access.