Skip to content

AsyncSRT.dispose()/worker.terminate() causes native heap corruption (reproduces on v1.5.5 and v1.5.6) #91

Open

Description

@birme

Summary

AsyncSRT.dispose() (which calls Worker.terminate() under the hood — see src/async.js) causes native heap corruption when called on a worker that has active or recently-closed SRT sockets. This reproduces on both v1.5.5 and v1.5.6 of the bundled libSRT — it is unrelated to the version-bump work in #89/#90.

This code path currently has no test coverage: dispose()/worker.terminate() is never called in spec/async_srt_await_spec.js (it's commented out) and is absent from every other spec file.

Reproduction

In a spec exercising AsyncSRT: create socket(s), close them via the async API, then call dispose() on the AsyncSRT instance (which calls Worker.terminate()) before the process exits.

Observed on Linux (node:20-bookworm, arm64, matching CI's toolchain):

free(): invalid pointer

and, on other runs of the same scenario:

free(): invalid next size (fast)

and one occurrence of a V8 fatal error during worker teardown:

Check failed: node->IsInUse().

All are consistent with a race between Worker.terminate() tearing down the worker thread/its Realm and the native addon's own in-flight close()/socket-teardown state still running inside that same worker thread — i.e. terminate() doesn't wait for the native binding's cleanup to finish before the thread (and its heap) is torn down.

Why this matters

Heap corruption (free(): invalid pointer / invalid next size) is a use-after-free/double-free class bug — exploitable in principle, not just a crash. Any application using AsyncSRT.dispose() for graceful shutdown (a reasonable, expected use of a public API method) can hit this today, independent of which libSRT version is bundled.

Suggested fix direction

Worker.terminate() is inherently abrupt — it doesn't let in-flight work finish. A safer dispose() likely needs to:

  • signal the worker to finish any pending native close()/socket teardown and acknowledge completion (e.g. a message round-trip) before calling worker.terminate(), or
  • avoid terminate() entirely in favor of a graceful shutdown message the worker acts on and then exits on its own.

Suggested test coverage

Un-comment and fix the existing dispose() call in spec/async_srt_await_spec.js, and add explicit close+dispose coverage to the other async spec files, so this path is exercised by CI going forward.


Found incidentally while investigating #89/#90 (libSRT v1.5.6 CVE fix); filed separately since it's unrelated to that work and affects the current release.

Activity

  1. added
    bugSomething isn't working
    ready-for-maintainerReviewed by triage bot; awaiting human merge
    on Aug 13, 2026
  2. birme commented on Aug 14, 2026

    @birme
    ContributorAuthor

    Thank you for this thorough and precise report, @birme. The analysis is exactly right, and the code confirms it.

    In src/async.js, dispose() calls Worker.terminate() — which is inherently fire-and-forget at the JS level. It sends a termination signal to the worker thread but does not wait for the native addon's own in-flight close()/socket teardown to complete before the thread and its associated heap are torn down. Any libSRT or N-API state that was still active inside that worker thread is then freed from underneath it, producing exactly the free(): invalid pointer / free(): invalid next size (fast) / Check failed: node->IsInUse() errors described here. These are classic use-after-free / double-free symptoms from a torn-down thread's heap.

    The impact is real and security-class: this is reachable through a documented, public API (AsyncSRT.dispose()) that users are expected to call for graceful shutdown. It is not a corner case.

    Fix directions (both valid):

    1. Graceful-shutdown message round-trip — before calling worker.terminate(), send the worker a shutdown message and await a reply that signals all native cleanup is complete, then call terminate(). This aligns with how other N-API addons handle cooperative teardown.
    2. Worker self-exit — have the worker drain its queue and exit on its own, avoiding terminate() entirely. Simpler in some designs.

    Either path requires touching src/async.js and src/async-worker.js (and possibly the N-API binding layer). A fix PR should also un-comment and extend the dispose() test coverage in spec/async_srt_await_spec.js and add explicit close+dispose coverage to the other async spec files, so this path is exercised by CI going forward.

    Because this involves the native addon's async/worker layer, any fix PR will be reviewed as Tier B and merged by a human maintainer.

    Marking this ready-for-maintainer given the security-class severity and the native-layer scope. A fix PR against master with the test coverage described would be very welcome.

    — node-srt maintenance bot

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingready-for-maintainerReviewed by triage bot; awaiting human merge

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions