BabylonJS / BabylonJS/JsRuntimeHost
UrlLib: Apple backend use-after-free when a UrlRequest is destroyed in flight
- Dominant language
- C++
- Stars
- 22
- Forks
- 23
- Avg merge
- 3d 14h
- Merged PRs (30d)
- 6
Description
Split out from review feedback on BabylonJS/UrlLib#37 (filed here because BabylonJS/UrlLib has Issues disabled).
## Problem
On the Apple (`NSURLSession`) backend, an in-flight `UrlRequest` that is destroyed before it settles leads to a use-after-free.
`UrlRequest_Apple.mm`'s `SendAsync()` builds a completion handler that writes `m_statusCode`, `m_headers`, `m_responseString` / `m_responseBuffer` and calls `SetError(...)` — so it implicitly captures a raw `this`. Nothing keeps the `Impl` alive for the duration of the task:
- `UrlRequest::Impl` there has no destructor.
- It never reads `m_cancellationSource`.
- It never calls `[task cancel]`.
`ImplBase::~ImplBase()` calls `Abort()`, but `Abort()` only does `m_cancellationSource.cancel()`, which this backend ignores. So destroying a `UrlRequest` neither stops the resumed `NSURLSessionDataTask` nor extends the impl's lifetime — the task keeps running and its handler later writes into freed memory.
## Impact
Any caller that drops a `UrlRequest` before it completes (e.g. abandoning a request on teardown) can corrupt whatever the freed allocation is reused for. This is not theoretical: it was observed in UrlLib CI, where an abandoned request's late failure handler wrote its error into a *subsequent* test's `Impl`, making an unrelated, previously-passing test report `NSURLErrorCancelled` (-999) with a non-empty `ErrorString`/`ErrorSymbol`.
## Notes
- Pre-existing; not introduced by #37. That PR's new test was simply the first code to exercise it, and now works around it by waiting for the request to settle before leaving scope.
- Worth auditing the other backends for the same pattern.
## Possible fixes
- Have `Impl` inherit `std::enable_shared_from_this` and capture a strong reference in the completion handler, so the impl outlives the task.
- And/or make `Abort()` actually cancel the `NSURLSessionDataTask` on this backend, honoring `m_cancellationSource`.
Contributor guide
Research direction
Start in UrlRequest_Apple.mm at Impl::SendAsync(), then trace ImplBase::~ImplBase() and Abort() to understand the existing cancellation path. Run the affected UrlLib CI coverage for abandoned in-flight requests. Done means destroying a request cannot let an Apple task access freed state, cancellation is honored, and the relevant backend behavior is covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100