Skip to content

Commit 2e8a89e

Browse files
bkaradzic-microsoftCopilot
authored andcommitted
Fix use-after-free in XMLHttpRequest::Send continuation
The completion continuation scheduled by Send captures `this` raw with no lifetime guarantee. If the JS XMLHttpRequest wrapper is garbage-collected while the request is in flight (e.g. once the requesting script drops its reference), the wrapper -- and the C++ object behind it -- is destroyed before the continuation runs on the runtime scheduler, so dereferencing m_request / m_eventHandlerRefs / m_readyState faults (access violation). Anchor the wrapper for the duration of the request with a strong Napi::ObjectReference held in a shared_ptr captured by the continuation, mirroring FileReader's existing anchor pattern. The anchor is released automatically once the request settles and the lambda is destroyed, so there is no member self-reference to clear and no extra teardown path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 000f5c9 commit 2e8a89e

1 file changed

Lines changed: 13 additions & 1 deletion

File tree

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,11 +259,23 @@ namespace Babylon::Polyfills::Internal
259259

260260
std::string traceName = (std::ostringstream{} << "XMLHttpRequest::Send [" << m_url << "]").str();
261261
auto sendRegion = std::make_optional<arcana::trace_region>(traceName.c_str());
262+
263+
// Keep the JS wrapper (and therefore this C++ object) alive for the
264+
// duration of the asynchronous request. The continuation below captures
265+
// `this` raw and dereferences members when the request settles; without
266+
// an anchor, GC may collect the wrapper while the request is in flight
267+
// (e.g. once the requesting script drops its reference) and the
268+
// continuation would then run on a freed `this`. The anchor lives in a
269+
// shared_ptr owned by the continuation lambda, so it is released
270+
// automatically once the request settles and the lambda is destroyed --
271+
// no member self-reference to clear. (Mirrors FileReader's anchor.)
272+
auto anchor = std::make_shared<Napi::ObjectReference>(Napi::Persistent(info.This().As<Napi::Object>()));
273+
262274
m_request.SendAsync()
263275
.then(arcana::inline_scheduler, arcana::cancellation::none(), [sendRegion{std::move(sendRegion)}]() mutable {
264276
sendRegion.reset();
265277
})
266-
.then(m_runtimeScheduler, arcana::cancellation::none(), [this](const arcana::expected<void, std::exception_ptr>& result) {
278+
.then(m_runtimeScheduler, arcana::cancellation::none(), [this, anchor{std::move(anchor)}](const arcana::expected<void, std::exception_ptr>& result) {
267279
// Run on every outcome -- transport exception OR underlying request succeeded but ended in a non-2xx
268280
// status (e.g. a missing local file on UWP, where UrlLib silently retains status 0). The previous
269281
// success-only continuation here skipped readyState=Done / loadend / error and let the JS observer

0 commit comments

Comments
 (0)