Fix path migration UAF: guard path promotion with InUse check - #6217
Merged
Conversation
Summary Fixes a use-after-free in QUIC path migration where a stale/removed path could be promoted into Paths[0] during post-processing. Root Cause When QuicConnReplaceRetiredCids is called during NEW_CONNECTION_ID frame processing, it may invoke QuicPathRemove, which frees the path. However, QuicConnRecvPostProcessing later attempts to promote that same path (via the *Path pointer) if it has non-probing frames and a new largest packet number — without verifying the path is still valid. Fix Add a (*Path)->InUse check in QuicConnRecvPostProcessing before promoting a path to active. This ensures we never promote a path that has already been removed by QuicPathRemove.
Anthony Rossi (anrossi)
approved these changes
Aug 11, 2026
Guillaume Hetier (guhetier)
enabled auto-merge (squash)
August 11, 2026 16:51
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6217 +/- ##
==========================================
- Coverage 86.03% 85.48% -0.56%
==========================================
Files 60 60
Lines 18981 18981
==========================================
- Hits 16330 16225 -105
- Misses 2651 2756 +105 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Guillaume Hetier (guhetier)
added a commit
that referenced
this pull request
Aug 11, 2026
…6217) (#6220) ## Description Fixes a use-after-free in QUIC path migration where a stale/removed path could be promoted into Paths[0] during post-processing. ### Root Cause When QuicConnReplaceRetiredCids is called during NEW_CONNECTION_ID frame processing, it may invoke QuicPathRemove, which frees the path. However, QuicConnRecvPostProcessing later attempts to promote that same path (via the *Path pointer) if it has non-probing frames and a new largest packet number — without verifying the path is still valid. ### Fix Add a (*Path)->InUse check in QuicConnRecvPostProcessing before promoting a path to active. This ensures we never promote a path that has already been removed by QuicPathRemove. ## Testing CI and validation on a repro of the issue ## Documentation N/A Co-authored-by: Sujeet Kumar <sujkum@microsoft.com>
Guillaume Hetier (guhetier)
added a commit
that referenced
this pull request
Aug 11, 2026
…6217) (#6219) ## Description Fixes a use-after-free in QUIC path migration where a stale/removed path could be promoted into Paths[0] during post-processing. ### Root Cause When QuicConnReplaceRetiredCids is called during NEW_CONNECTION_ID frame processing, it may invoke QuicPathRemove, which frees the path. However, QuicConnRecvPostProcessing later attempts to promote that same path (via the *Path pointer) if it has non-probing frames and a new largest packet number — without verifying the path is still valid. ### Fix Add a (*Path)->InUse check in QuicConnRecvPostProcessing before promoting a path to active. This ensures we never promote a path that has already been removed by QuicPathRemove. ## Testing CI and validation on a repro of the issue ## Documentation N/A Co-authored-by: Sujeet Kumar <sujkum@microsoft.com>
Guillaume Hetier (guhetier)
added a commit
that referenced
this pull request
Aug 12, 2026
…6217) (#6218) ## Description Fixes a use-after-free in QUIC path migration where a stale/removed path could be promoted into Paths[0] during post-processing. ### Root Cause When QuicConnReplaceRetiredCids is called during NEW_CONNECTION_ID frame processing, it may invoke QuicPathRemove, which frees the path. However, QuicConnRecvPostProcessing later attempts to promote that same path (via the *Path pointer) if it has non-probing frames and a new largest packet number — without verifying the path is still valid. ### Fix Add a (*Path)->InUse check in QuicConnRecvPostProcessing before promoting a path to active. This ensures we never promote a path that has already been removed by QuicPathRemove. ## Testing CI and validation on a repro of the issue ## Documentation N/A Co-authored-by: Sujeet Kumar <sujkum@microsoft.com>
Masahiro Kozuka (masa-koz)
added a commit
to seera-networks/msquic
that referenced
this pull request
Aug 14, 2026
## Description Merge `main` (tracking upstream microsoft/msquic) into `seera-main`. 25 upstream commits since the last sync at `c2271990`. Notable ones for the fork: | Upstream | What it changes | |---|---| | microsoft#6217 | Path migration UAF — guards path promotion with an `InUse` check | | microsoft#6213 | Clamps the advertised `MaxAckDelay` to `MinAckDelay` | | microsoft#6047 | Discards client Initial keys when Handshake write keys are installed; adds `QUIC_PACKET_BUILDER::InitialKeysDiscarded` so a flush that frees allowance reschedules | | microsoft#6167 | Validates the Version Negotiation packet's Source CID against our Destination CID | | microsoft#6181 | Handles zero-length datagrams in epoll and io_uring | | microsoft#6198 | Upgrades the XDP dependency to v1.4 | | microsoft#6199 | Removes the obsolete Linux raw datapath (`datapath_raw_linux.c`, `datapath_raw_socket_linux.c`) and their clog artifacts | | microsoft#6155 | Shifts Linux OpenSSL CI to Ubuntu 26.04, and adds `NOLINTNEXTLINE` clang-tidy suppressions across core | Submodules move with it: googletest `fa005b2`→`c0afb7f`, openssl `1d6826b`→`3d9ddf4`, xdp-for-windows to v1.4. ### Conflicts Four, all resolved keeping the fork's behaviour and taking upstream's change where the two were independent. **`src/core/bbr.c`** — upstream added a `NOLINTNEXTLINE(clang-analyzer-security.ArrayBound)` above `QuicPathGetDatagramPayloadSize(&Connection->Paths[0])`. The fork reads `PathID->Path` there instead, since congestion control is per path ID under multipath, and has no `Connection` local at all. Kept the fork's expression with upstream's suppression above it. **`src/core/crypto.c`** — upstream added the same kind of suppression above `Connection->State.HandshakeConfirmed = TRUE`; the fork had added an early return when it is already set. Independent, so both are kept, guard first. Both suppressions are needed, and the one in `crypto.c` had to move. The analyzer reports at the *first* access to the containing record, which in the fork's `crypto.c` is the guard, not the assignment upstream put their comment above; `NOLINTBEGIN`/`NOLINTEND` now covers both statements so it does not have to be revisited if the reported line shifts. Verified by running the CI's own toolchain — clang-tidy 21.1.8, from `ghcr.io/microsoft/msquic/linux-build-xcomp:ubuntu-26.04-cross` — over every source file this fork changes relative to upstream: all 26 pass, and removing either suppression reproduces the exact CI error. **`.github/workflows/test.yml`, `.github/workflows/stress.yml`** — upstream bumped the Windows XDP jobs from `xdp-v1.1` to `xdp-v1.4` and, in the same region, added `WinServerPrerelease` matrix entries. Took the XDP bump; did not reintroduce `WinServerPrerelease`, which #14 removed from this fork because those jobs need Microsoft's self-hosted `1ES.Pool=1es-msquic-pool`. The fork's `windows-2025` substitutes for the two `Full.Verbose` schannel jobs are kept. Upstream's deletion of the openssl+XDP recvfuzz entry is taken as a plain deletion. After the merge no `xdp-v1.1` reference and no `WinServerPrerelease` matrix entry remains. ## Testing | Sweep | Result | |---|---| | `*Multipath*:*Path*:*Migration*:*UnconnectedSocket*:*KeepAlive*` | 109 tests pass | | `*Basic*:*Datagram*:*Receive*:*Recv*:*Handshake*` | 986 tests pass | Build is clean, no compiler warnings. The first push failed the six `-CodeCheck` jobs on a single diagnostic at `crypto.c:490`, fixed in the follow-up commit. Local clang-tidy is 18 and does not reproduce it at all, which is why the merge commit shipped with the suppression in the wrong place; the CI container was used for verification afterwards. Spot-checked that the fork's own work survived the merge intact: the per-path keep alive (`Path->LastSendTimeUs` stamped in `QuicPacketBuilderSendBatch`, read by `QuicConnPathKeepAliveTimerUpdate`), and that microsoft#6217's active-path promotion guard sits inside the existing `!MultipathNegotiated` branch so it does not affect multipath connections. ## Documentation No documentation change of its own. `docs/Settings.md`, `docs/XDP.md`, `docs/Release.md` and `docs/TEST.md` move with upstream. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Masahiro Kozuka (masa-koz)
added a commit
to masa-koz/msquic-async-rs
that referenced
this pull request
Aug 14, 2026
## Summary Bumps the `seera-msquic` submodule `5b21c4f` → `146714f`. Unlike the recent bumps this is not a fork change: it is `seera-main` taking **25 commits of upstream microsoft/msquic** ([seera#77](seera-networks/msquic@146714f)), plus one fork commit adjusting a static-analysis suppression the merge displaced. 106 files, +544/-5355 — most of the deletions are obsolete Linux raw datapath files and Azure pipeline definitions upstream removed. **Nothing in this crate needed changing.** `src/rs` is untouched, so the Rust bindings and every signature this crate calls are unchanged; the only public header change is one line in `msquic.hpp`, which is C++ and unused here. ## What is worth knowing Upstream fixes that reach this crate's users: - **Path migration use-after-free** ([#6217](microsoft/msquic#6217)). `QuicConnRecvPostProcessing` promoted a path on a non-probing frame without checking `(*Path)->InUse`, so a packet on a stale path entry could promote freed state. A one-line guard — and it is in the path handling this project exercises through `Connection::add_path()`. - Zero length datagrams are handled in the epoll and iouring datapaths ([#6181](microsoft/msquic#6181)). - `MinAckDelay` is clamped to `MaxAckDelay` in the local transport parameters ([#6213](microsoft/msquic#6213)). - Client Initial keys are discarded when Handshake write keys are installed ([#6047](microsoft/msquic#6047)), and VN packet source CIDs are validated ([#6167](microsoft/msquic#6167)). - `build.rs` no longer fails to link on lib64 distros ([#6168](microsoft/msquic#6168)) — about building this crate rather than running it. The fork commit `f465ff0` moves a clang-analyzer `ArrayBound` suppression in `crypto.c` onto the fork's early return, which is where the analyzer now reports because that guard reads `Connection` one statement before upstream's assignment. Build-only. ## Nested submodules The merge moves seera-msquic's own submodules: **openssl** `1d6826b` → `3d9ddf4`, googletest `fa005b2` → `c0afb7f`, xdp-for-windows `ccd59c2` → `d372b52`. A checkout that only updates the top-level submodule will build the new source against the *old* openssl, so this bump wants `git submodule update --init --recursive` rather than a plain `git submodule update`. Both were updated here before building. ## Test plan - `cargo test -p msquic-async --lib --no-default-features --features tokio,msquic-seera` — 43 passed, on a full rebuild of the vendored C library and the updated openssl, not a cached one 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Masahiro Kozuka (masa-koz)
added a commit
to seera-networks/ISEKAI-link
that referenced
this pull request
Aug 15, 2026
`4722f6c..5b0144f` on `masa-koz/qmux-01-fix-h3-datagram`. Three commits, and the substance is a `seera-msquic` bump (`bdf732d..8c3e201`) merging 25 commits of upstream microsoft/msquic. ## The one that is ours **A use-after-free in path migration** ([msquic#6217]). `QuicConnRecvPostProcessing` promoted a path on a non-probing frame without checking `(*Path)->InUse`, so a packet arriving on a stale path entry could promote freed state. That is the code this repository exercises through `add_path`, on every connection holding a relay path and a direct one at once — which is all of them since #95 stopped switching between the two and started keeping both. [msquic#6217]: microsoft/msquic#6217 ## The rest Upstream hygiene that reaches us regardless: zero-length datagrams handled in the epoll and iouring datapaths, `MinAckDelay` clamped to `MaxAckDelay` in the local transport parameters, client Initial keys discarded when Handshake write keys are installed, VN packet source CIDs validated, and a build fix for lib64 distros. The third commit syncs the submodule's own `Cargo.lock` to the h3 revision its branch pins. **Nothing to do on this side** — `rust/Cargo.lock` has been on `c916ed5` since #101; the submodule's copy was the one lagging. ## Checked The interesting part of this change is C, so checking it needed care. `cargo check` passed in five seconds and proved nothing: the build script does not rerun on submodule sources, so `libmsquic.so` was still the one built on 20 July. After `cargo clean -p seera-msquic` it rebuilt and linked — 9,139,304 → 9,423,200 bytes, today's timestamp — which also exercises the merge's `scripts/build.rs` conflict resolution, the one that touches the link search path. Then `cargo test --workspace --lib --bins`: `isekai-p2p` 39, `isekai-p2p-core` 34, `camera-core` 22, `camera-server` 5 and 1 ignored, and the rest. ## What is not covered `cargo test --workspace` — without `--lib` — **hangs**, in `camera-core/tests/video_loopback.rs`. Not this change: three abandoned runs of that same test binary are still alive on the build machine from 2 and 3 August, which is eleven days before this bump. CI has never noticed because `migration-spike.yml` runs `cargo test -p camera-core --lib`, and `--lib` excludes `tests/`. Being reported separately. It means the loopback path has no working test coverage on either side of this change, so what this bump rests on is the unit tests, the link, and the CI builds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes a use-after-free in QUIC path migration where a stale/removed path could be promoted into Paths[0] during post-processing.
Root Cause
When QuicConnReplaceRetiredCids is called during NEW_CONNECTION_ID frame processing, it may invoke QuicPathRemove, which frees the path. However, QuicConnRecvPostProcessing later attempts to promote that same path (via the *Path pointer) if it has non-probing frames and a new largest packet number — without verifying the path is still valid.
Fix
Add a (*Path)->InUse check in QuicConnRecvPostProcessing before promoting a path to active. This ensures we never promote a path that has already been removed by QuicPathRemove.
Testing
CI and validation on a repro of the issue
Documentation
N/A