Skip to content

Audit invokeMethod+[this] captures for queued-connection use-after-free risk #2990

Description

@ten9876

Context

PR #2946 fixed a confirmed use-after-free in `TciProtocol` queued lambdas (#2814) — the protocol was being synchronously deleted while `Qt::QueuedConnection` lambdas still held a `this` pointer to it.

The same pattern (`QMetaObject::invokeMethod(target, this, ... { ... }, Qt::QueuedConnection)` where the capturing object can be destroyed across thread boundaries) likely exists elsewhere in the codebase. This issue tracks an audit pass.

Candidate sites from a quick grep

Quick `grep -rn 'invokeMethod.*[this' src/` after #2946 lands turns up roughly 30 callsites. Most are probably safe — `this` is typically a long-lived QObject (MainWindow, RadioModel, AudioEngine, all owned by MainWindow). But two clusters look like potential mirrors of the TciProtocol pattern:

🔴 `src/core/RigctlProtocol.cpp` — five `[this, ...]` queued captures

Lines 392, 591, 686, 697, 710. RigctlProtocol's relationship to RigctlServer needs the same lifetime analysis the PR did for TciProtocol/TciServer:

  • Is RigctlProtocol synchronously deleted when a rigctl client disconnects?
  • If so, are these captures vulnerable to the same UAF window?
  • Same fix would apply: `[model = m_model, ...]` capture.

This is the highest-priority site because the structural parallel to TciProtocol is exact.

🟡 `src/core/TciServer.cpp` — four `[this, ...]` captures (lines 358, 376, 730, 739)

These capture TciServer's `this`, not TciProtocol's. TciServer outlives the app (owned by MainWindow). So they're probably safe. Worth verifying that TciServer is never destroyed mid-app-lifecycle in any path (e.g., re-init on connection settings change).

🟢 Other invokeMethod+[this] sites that are probably safe

  • `src/core/AudioEngine.cpp` (8 sites): `this` = AudioEngine, owned by MainWindow, lives the whole app session
  • `src/core/NvidiaBnrFilter.cpp` (2 sites): `this` is a long-lived filter object
  • `src/core/SerialPortController.cpp` (1 site): in a worker-thread context, controller outlives the lambda
  • `src/gui/SpectrumWidget.cpp` (1 site): widget queued back to itself
  • `src/gui/RadioSetupDialog.cpp` (4 sites): modal dialog, lives until the user closes it
  • `src/gui/AetherDspWidget.cpp` (1 site)
  • `src/models/RadioModel.cpp` (2 sites): RadioModel queues to its connection — both long-lived
  • `src/models/DaxIqModel.cpp` (2 sites)

These should be confirmed but the structural risk is much lower because the captured object has a clear "owned by MainWindow, outlives the app" lifetime.

Recommended audit steps

For each `[this]` queued-connection capture:

  1. Identify the owning object's lifetime — is it ever synchronously destroyed while in a context where queued lambdas could still be in flight?
  2. If yes (like TciProtocol/RigctlProtocol-style per-connection objects): convert to capture-by-value of the actually-needed members (`m_model`, etc.), or use a QPointer guard inside the lambda.
  3. If no (long-lived MainWindow-owned objects): document the lifetime invariant in a code comment so future refactors don't break it silently.

Why this matters beyond #2814

UAFs in queued lambdas are particularly nasty because:

  • They're non-deterministic — only manifest when the lambda races the destruction
  • The stack trace shows the lambda fire site, not the dangling pointer cause
  • Stress testing (rapid connect/disconnect) is what surfaces them; normal use doesn't

PR #2946 had ASan to catch it because the reporter was running the exact tune-on/off repro that timed the race. Other sites may have the same bug latent — they'd just need a different repro to surface.

Effort

~half day for the full audit. RigctlProtocol fix alone (if confirmed vulnerable) is ~1 hour following the TciProtocol pattern from #2946.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingmaintainer-reviewRequires maintainer review before any action is takenpriority: mediumMedium priorityprotocolSmartSDR protocol

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions