On Sat, Sep 26, 2026 at 10:45:54PM -0400, andrew gonzalez wrote: > This is a hardening change, not a security fix — I want to be clear > about that up front. > > `buf_len()` looks like it tolerates an unusable buffer (it returns 0 > when `buf_valid()` says no), but `buf_valid()` dereferences the > pointer before testing anything, so `buf_len(NULL)` faults inside its > own validity check. Eight helpers in `buffer.h` gate on `buf_valid()`. > > bc7f77ea fixed a pre-auth SIGSEGV whose proximate cause was exactly > `buf_len(NULL)` on `tls_crypt_v2_wkc`. I checked that fix and it is > complete — `CO_RESEND_WKC` has a single setter and it is now guarded, > so both sinks are unreachable today. This patch just removes the edge > for the next caller. > > The `likely()` markers keep the hot path unchanged. The regression > test is validated rather than vacuous: with only `buffer.h` reverted, > `buffer_testdriver` reports 22 run / 21 passed / 1 failed > (`test_buffer_null_predicates`); with the patch applied it is 22/22. > On current master, all 16 test drivers pass on macOS. > > Happy to drop the `buf_defined()` half, or to narrow this to > `buf_len()` only, if you would rather not add a branch to > `buf_valid()`.
LGTM. Acked-by: Frank Lichtenheld <[email protected]> Regards, -- Frank Lichtenheld _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
