TL;DR: A batch of client-side memory-safety bugs in libnfs and libsmb2 — the NFS and SMB client libraries used by KDE, Kodi, and a long tail of media/storage software — went nowhere for two weeks, then got fixed within hours of a single follow-up email. The bugs themselves were unremarkable heap over-reads. The parts worth writing down are the disclosure mechanics and the two moments where verification, not discovery, saved the report: one bug had already been fixed upstream before I re-sent it, and another looked fixed when it wasn’t because of how the harness was built.
The findings
All of these are reached from a malicious or MITM’d server — the client parses the server’s reply, so a hostile server (or an attacker on the path) controls the bytes. That’s the important framing: people think of NFS/SMB clients as trusted, but the reply parser is an attack surface.
- libsmb2 —
ntlm_decode_challenge_message()read a fixed 56-byte NTLM CHALLENGE header without checking the message was that long (pre-auth). - libnfs —
libnfs_zdr_opaque()memcpy’d a fixed-length opaque field with no bounds check, unlike every sibling ZDR primitive. - libnfs —
nfs_parse_attributes()advanced past the owner/group strings in a GETATTR reply but forgot to shrink its remaining-length counter, so later bounds checks passed against an inflated budget. - libsmb2 — the FILESYSTEM_INFO decoders fed an attacker-controlled label/name length straight to a UTF-16 conversion with no bound against the received reply.
- libsmb2 — an integer overflow in
ndr_decode_utf16()’s bounds check.
Reported by PGP-signed email to the maintainer, Ronnie Sahlberg, per the projects’ disclosure norms.
The silence, and the nudge
The first two went out on 10 July. Then: nothing. No acknowledgement for over two weeks. This is the normal, unglamorous middle of coordinated disclosure — a solo-maintained OSS project is not an inbox anyone watches daily.
The move that worked was a dated re-confirmation, not a chase. On 27 July I
sent a short follow-up per finding: “I retested the original reproducer against
current master <commit> today, and AddressSanitizer still reports the overflow.”
No deadline theatrics — just evidence the bug was still live, with the exact commit
I’d checked. Within hours the maintainer replied and pushed fixes to master for
both. Persistence beats pressure; a fresh, specific retest gives a busy maintainer
everything they need to act in one sitting.
Verification save #1: don’t report a bug that’s already fixed
Before sending the second batch (the GETATTR and FILESYSTEM_INFO bugs, which had been packaged but not yet disclosed), I did what the playbook demands: recheck live upstream immediately before reporting.
Good thing. One of the three queued findings — the ndr_decode_utf16() integer
overflow — had been fixed upstream in the meantime, incidentally, during a
refactor that split the DCE/RPC code into a separate library. The current code
already had the exact 64-bit-cast guard I was about to suggest. Re-sending it would
have been a credibility hit with a maintainer who’d just started engaging — instead
the bundle noted it was already resolved, which reads as diligence. The recheck
costs a few minutes; skipping it costs trust.
Verification save #2: the bug that looked fixed but wasn’t
The FILESYSTEM_INFO fix is where I nearly shipped something wrong in the other direction. Building the fix against current master, my reproducer came back clean even without the fix applied — as if the bug were gone.
It wasn’t. The over-read happens inside smb2_utf16_to_utf8() — a library
function. My harness was compiled with AddressSanitizer, but I’d linked it against
a libsmb2 built by its normal CMake config, without ASan. ASan only catches
memory accesses in instrumented code, so the out-of-bounds read, executing in the
uninstrumented library, sailed straight through undetected.
Rebuilding the whole library with -fsanitize=address made it reproduce
instantly: crash without the fix, clean with it. The lesson generalises: your
sanitizer has to instrument the code that actually performs the vulnerable
operation. A related finding in the same session (an uninitialised-read leak in
libgsf) had the mirror-image trap — ASan can’t see uninitialised reads at all;
that one needed valgrind. Know your tool’s blind spots, and confirm a bug
reproduces both directions (crashes before the fix, clean after) before you trust
it.
Landing the fixes: PRs for credit
For the last two, the maintainer’s preference was telling: “Please send them as PRs, that way you can adjust the credit for fixing it in the commit message.” So they went in as pull requests, each fix built and verified against current master, with the bug rationale in the PR body — merged the same day (libnfs #593, libsmb2 #471).
One last bit of noise worth naming: both PRs tripped CI failures — but on the Windows and AmigaOS build jobs, which were already red on the previous commit and have nothing to do with a three-line C bounds check. The Linux builds and tests passed. Don’t panic at a red check; read which check, and whether it was red before you touched anything.
Takeaways
- Client reply-parsers are an attack surface. “It’s just a client” is not a threat model; a malicious server controls every byte the client parses.
- A dated retest beats a nagging reminder. Give the maintainer fresh, specific evidence and a one-sitting path to a fix.
- Recheck upstream right before you report. The bug may already be fixed; re-reporting it costs trust.
- Confirm both directions, and instrument the right code. A “clean” result can be a false negative — from an uninstrumented library, or a bug class your sanitizer doesn’t model at all.