nstun: bounds-checked cursor for peer-controlled packet parsing + libFuzzer/OSS-Fuzz coverage
- Dominant language
- C++
- Stars
- 4.1k
- Forks
- 371
- Avg merge
- 4d 7h
- Merged PRs (30d)
- 7
Description
Hi — before I send a larger PR, CONTRIBUTING asks me to check in first.
nstun parses IPv4/IPv6 headers, the IPv6 extension-header chain, ICMP/TCP/UDP,
and the SOCKS5/HTTP CONNECT proxy replies — all off bytes the peer controls.
Today each site keeps a raw `(const uint8_t*, size_t)` pair in sync by hand and
`reinterpret_cast`s the L4 header after its own length check. The bounds are
correct, but the invariant is maintained only by convention.
I'd like to send two small, separable changes:
1. **A bounds-checked cursor** (`ByteReader`, a thin wrapper over one
`std::span`) so the pointer and remaining length are the same object and the
accessors return `nullptr`/`false` instead of ever reading past the end. It
routes the IPv4/IPv6 receive paths through it and moves the extension-header
walk into a dependency-free `ipparse.{h,cc}`. No functional change — the set
of accepted/rejected packets is identical. I rebased it over your Aug 26
SSRF-gate change (1a923df, the IPv6 unspecified + multicast drops) and kept
those checks intact.
2. **libFuzzer targets** for the extension-header walk and the four proxy-reply
parsers, plus an `infra/oss-fuzz/build.sh` so nstun's packet parsers can get
OSS-Fuzz coverage (they have none today). The targets link only the
dependency-free parsing TUs, so `make fuzz` works without protobuf/libnl.
Separately I have a build-hardening change (full RELRO, stack protector,
`_FORTIFY_SOURCE=3`, CET/BTI via cc-option probing) that I'd send on its own
since it touches global build flags.
Does this direction sound welcome, and do you prefer the cursor and the fuzzers
as one PR or two? Happy to adjust to house style before sending.
Contributor guide
Research direction
Read CONTRIBUTING and inspect the existing IPv4/IPv6 receive paths, extension-header walk, and SOCKS5/HTTP CONNECT reply parsers. Use the proposed ipparse.{h,cc} and infra/oss-fuzz/build.sh locations as entry points; done means the parsing paths use the bounds-checked cursor without changing accepted packets, and the fuzz targets build with make fuzz.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- networking, security, testing-qa
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100