microsoft / microsoft/mssql-rs
mssql-odbc: LIVE_HANDLES stale-handle registry is vulnerable to address reuse (ABA)
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
### Context
PR #415 (fixing #400 and #401) added `mssql-odbc/src/handles/mod.rs`'s `LIVE_HANDLES` registry — a process-wide `HashSet` of currently-allocated handle addresses — so `free_stmt`/`free_desc` (and `SQLDisconnect`'s statement/descriptor cleanup loops) can confirm a handle is still live *before* dereferencing it, instead of dereferencing first.
This closes the specific, reproducible bug in #400: a handle freed once by `SQLDisconnect`'s cascade, then legitimately (from the caller's point of view) freed again later by the application, with no intervening reallocation at that address. That's exactly the scenario #400's repro and the macOS CI failure hit.
### The gap
The registry tracks raw addresses, not allocation identity. It answers "is *some* handle currently allocated at this address," not "is this *specific* allocation still the one at this address." Concretely:
1. Handle A is allocated at address `P`, later freed (removed from `LIVE_HANDLES`).
2. The allocator reuses `P` for a new, unrelated handle B (inserted into `LIVE_HANDLES` under the same key `P`).
3. An application still holding a stale reference to A calls `SQLFreeHandle` (or similar) on `P`.
4. `is_live(P)` now reports `true` — not because A is valid, but because the registry cannot distinguish A from B at the same address.
5. The call proceeds to dereference/free `P` as if it were still A, corrupting or freeing B instead — a real use-after-free/type-confusion, just narrower in scope than before this registry existed (it now only misfires on the address-reuse sub-case, not on every stale handle).
This was flagged by Copilot's automated review on PR #415 and is documented in detail in the doc comment on `LIVE_HANDLES` in `mssql-odbc/src/handles/mod.rs`, along with a test (`handles::tests::is_live_cannot_distinguish_a_reused_address_from_the_original_allocation`) that demonstrates the exact mechanism directly against the registry (real OS-level address reuse can't be forced portably/deterministically from a test).
### Why not fixed in #415
Actually closing this needs handle identity that survives address reuse — e.g. a generation-tracked indirection layer (handles as small stable indices/generations into a slot table, rather than raw pointers) instead of a raw-address registry. That is the same class of redesign as the existing "refcounted handle lifetimes" TODO already called out in `mssql-odbc/src/api/disconnect.rs`, and is out of scope for a narrow bug-fix PR.
### Suggested follow-up
Design and implement a generational/stable-index handle identity scheme (or equivalent) that remains valid across address reuse, likely alongside (or as part of) the refcounted-handle-lifetime redesign already tracked informally in `disconnect.rs`. This should subsume `LIVE_HANDLES` once done.
### Severity / urgency
Low: requires a caller to (a) retain and reuse a stale handle after it was freed via the `SQLDisconnect` cascade path, and (b) race the allocator reusing that exact address in between — a narrow window, and the registry still strictly improves on the pre-#415 behavior (unconditional dereference with no check at all) for the far more common non-reuse case.
Contributor guide
Assessment
This issue has not been assessed yet.