Skip to content

build(deps): bump github.com/insomniacslk/dhcp past the nclient4 ReadFrom panic fix - #15

Closed
thc1006 wants to merge 1 commit into
ze-software:mainfrom
thc1006:bump-insomniacslk-dhcp-583
Closed

build(deps): bump github.com/insomniacslk/dhcp past the nclient4 ReadFrom panic fix#15
thc1006 wants to merge 1 commit into
ze-software:mainfrom
thc1006:bump-insomniacslk-dhcp-583

Conversation

@thc1006

@thc1006 thc1006 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

The in-process DHCPv4 client runs during network bring-up. internal/plugins/iface/dhcp/dhcp_v4_linux.go calls nclient4.New at lines 38 and 129, and the installer path internal/install/disk/dhcp_linux.go:36 calls it too. None pass WithUnicast, so they're default broadcast clients reading through BroadcastRawUDPConn.ReadFrom.

Before insomniacslk/dhcp#583, a frame whose IPv4 payload was too small to hold a UDP header made nclient4 read past the frame buffer and panic the receive loop. The fix adds if ipPayloadLen < udpHdrLen { continue }; you can see the guard in the vendored conn_unix.go in this PR. The pin here (v0.0.0-20260326115832-991c7910cf36) predates it. This moves it to the merge commit and re-vendors. go mod vendor and go build ./... pass. I wrote the fix upstream. No CVE, and I haven't tried to trigger the panic in ze.

A note on process: this is a maintenance and security bump, not feature work, so I skipped the issue-and-spec flow in CONTRIBUTING, the same way the Dependabot go-modules PRs do. Dependabot can't pick this one up: insomniacslk/dhcp has no tags, only pseudo-versions, and Dependabot doesn't propose commit-to-commit bumps for those, which is why the pin stayed at the March commit through the weekly runs. The vendored changes outside conn_unix.go are upstream's own refactor between the two commits, not mine, and go mod vendor reproduces the tree with zero drift, which I checked.

@thc1006 thc1006 changed the title go.mod: bump insomniacslk/dhcp past the nclient4 ReadFrom panic fix build(deps): bump github.com/insomniacslk/dhcp past the nclient4 ReadFrom panic fix Aug 16, 2026
@thomas-mangin

Copy link
Copy Markdown
Contributor

Thank you for the report, I have not yet finished the code and I am going through ensuring we have test for all the MUST statement of the RFC. This work is still ongoing.

Before release I also need to add some more debugging and run adversarial pass on the code by AI.

Your report is correct about fault, so I will merge it and then work on it.

Thank you

Picks up insomniacslk/dhcp#583, which guards BroadcastRawUDPConn.ReadFrom against frames whose IPv4 payload cannot hold a UDP header. Target: v0.0.0-20260719225207-c76316d4aa82.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006
thc1006 force-pushed the bump-insomniacslk-dhcp-583 branch from 4ab72ff to 359ddbc Compare August 17, 2026 19:02
@thomas-mangin

thomas-mangin commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

My Friendly AI says:

Merged as cd9856f on main, with your authorship and sign-off kept. It was replayed onto current main rather than merged here: the branch had drifted 110 commits behind and go.mod conflicted, so the pin was reapplied and the tree re-vendored. The result is byte-identical to this pull request, all 8 files.

Two notes from the review, both useful to you.

Your description understates the impact. nclient4 runs receiveLoop on a goroutine it creates itself (client.go:223), so ze's own recover around the DHCP worker never sees the panic and the whole process dies. On the installer path that process is PID 1 in the initrd. The frame is reachable from the link: an IPv4 header declaring four payload bytes, eight crafted bytes that read as a UDP header for port 68, padded past the length the header claims. That makes this an unauthenticated on-link crash, not only a robustness fix.

Your zero-drift check was true on your base and is no longer true on main. go mod vendor there reverts a local netlink XFRM patch, which has to be reapplied with git apply scripts/dev/patches/netlink-xfrm-fixes.patch. Worth knowing before the next vendor bump.

Follow-up landed in 7531c4a: a QEMU integration test that drives the real client over a veth pair and asserts it skips the short frame and still reads the OFFER, plus a guard test that refuses a re-vendor which drops the fix. Both were measured against the old vendored file; the integration one panics there with slice bounds out of range [:-4]. Vendoring your upstream conn_unix_test.go would not have helped, since go mod vendor drops _test.go files and no ze gate matches vendor packages.

Thank you for the fix, upstream and here.

@thc1006
thc1006 deleted the bump-insomniacslk-dhcp-583 branch August 17, 2026 19:14
@thc1006

thc1006 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks Thomas. Appreciate you replaying it onto main and keeping my authorship. You're right that I undersold the impact. The receiveLoop runs on its own goroutine, so ze's recover never sees the panic, and on the installer path that takes down PID 1. Good to know about the netlink XFRM patch, and the QEMU veth test plus the re-vendor guard are a nice touch. Thanks again for the careful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants