Fix UAF crash from dangling output signal handlers (#1716)

* Fix UAF crash from dangling output signal handlers

osn::Output connected the "stop" handler and per-signal
OutputSignalCallback (each with a heap CallbackData carrying `this`)
but never disconnected them or freed the CallbackData. When the
underlying obs_output_t outlived the wrapper -- delay buffer, reconnect,
or a shared SetOutput ref -- a later signal fired into freed
CallbackData/this, crashing in signal_handler_signal (seen in Sentry as
EXCEPTION_ILLEGAL_INSTRUCTION via ISimpleReplayBuffer::Stop ->
obs_output_stop). Heavy start/stop churn (e.g. recording repeatedly
failing to write) widened the window.

- Add DisconnectSignals(): disconnects OnStopped and every
  OutputSignalCallback, then frees the CallbackData (also fixes the
  pre-existing leak).
- Track allocated CallbackData in m_signalCallbackData.
- Promote the anonymous "stop" lambda to a named static OnStopped so it
  has a stable address to disconnect.
- Call DisconnectSignals() in DeleteOutput after the stop-wait and before
  obs_output_release, so OnStopped stays live during the wait and no
  connection survives the release.

All output types tear down through DeleteOutput (Simple/Advanced ->
Streaming/Recording/ReplayBuffer dtors, plus CreateOutput/SetOutput
recreate), so this covers every path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Make ConnectSignals private

Only called internally from InitOutput; addresses review feedback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: summeroff

obs-studio-server/source/osn-output.cpp

@@ -20,15 +20,6 @@
 
 #include "nodeobs_api.h"
 
-namespace {
-
-struct CallbackData {
-	std::string signal;
-	osn::Output *outputClass;
-};
-
-} // namespace
-
 osn::Output::Output(const std::vector<std::string> &signals) : m_signals(signals) {}
 
 osn::Output::~Output() {}
@@ -40,19 +31,20 @@ void osn::Output::InitOutput(obs_output_t *output)
 		m_outputStopped = false;
 	}
 
-	auto onStopped = [](void *data, calldata_t *) {
-		osn::Output *context = reinterpret_cast<osn::Output *>(data);
-		std::unique_lock lock(context->m_mtxOutputStop);
-		context->m_outputStopped = true;
-		context->m_cvStop.notify_one();
-	};
-
 	signal_handler *sh = obs_output_get_signal_handler(output);
-	signal_handler_connect(sh, "stop", onStopped, this);
+	signal_handler_connect(sh, "stop", osn::Output::OnStopped, this);
 
 	ConnectSignals();
 }
 
+void osn::Output::OnStopped(void *data, calldata_t *)
+{
+	osn::Output *context = reinterpret_cast<osn::Output *>(data);
+	std::unique_lock lock(context->m_mtxOutputStop);
+	context->m_outputStopped = true;
+	context->m_cvStop.notify_one();
+}
+
 void osn::Output::CreateOutput(const std::string &type, const std::string &name)
 {
 	DeleteOutput();
@@ -89,13 +81,19 @@ void osn::Output::DeleteOutput()
 			blog(LOG_WARNING, "Timed out waiting for output stop before release.");
 		}
 	}
+
+	// Disconnect before release: the obs_output_t can outlive this object
+	// (delay buffer, reconnect, or a shared ref), and a later signal firing
+	// into freed CallbackData/this would crash in signal_handler_signal.
+	DisconnectSignals();
+
 	obs_output_release(m_output);
 	m_output = nullptr;
 }
 
 void osn::OutputSignalCallback(void *data, calldata_t *params)
 {
-	auto info = reinterpret_cast<CallbackData *>(data);
+	auto info = reinterpret_cast<osn::Output::CallbackData *>(data);
 
 	if (!info)
 		return;
@@ -122,10 +120,28 @@ void osn::Output::ConnectSignals()
 		auto *cd = new CallbackData();
 		cd->signal = signal;
 		cd->outputClass = this;
+		m_signalCallbackData.push_back(cd);
 		signal_handler_connect(handler, signal.c_str(), osn::OutputSignalCallback, cd);
 	}
 }
 
+void osn::Output::DisconnectSignals()
+{
+	if (!m_output)
+		return;
+
+	signal_handler *handler = obs_output_get_signal_handler(m_output);
+	if (handler) {
+		signal_handler_disconnect(handler, "stop", osn::Output::OnStopped, this);
+		for (auto *cd : m_signalCallbackData)
+			signal_handler_disconnect(handler, cd->signal.c_str(), osn::OutputSignalCallback, cd);
+	}
+
+	for (auto *cd : m_signalCallbackData)
+		delete cd;
+	m_signalCallbackData.clear();
+}
+
 void osn::Output::StartOutput()
 {
 	if (!m_output)

obs-studio-server/source/osn-output.hpp

@@ -39,7 +39,6 @@ class Output {
 	Output(const std::vector<std::string> &signals);
 	virtual ~Output();
 
-	void ConnectSignals();
 	void CreateOutput(const std::string &type, const std::string &name);
 	void SetOutput(obs_output_t *output);
 	virtual void DeleteOutput();
@@ -63,8 +62,24 @@ class Output {
 private:
 	friend void OutputSignalCallback(void *data, calldata_t *params);
 
+	// One per connected signal; carries the signal name and owner so the
+	// forwarding callback can identify them. Heap-allocated in ConnectSignals
+	// and freed in DisconnectSignals.
+	struct CallbackData {
+		std::string signal;
+		osn::Output *outputClass = nullptr;
+	};
+
 	void InitOutput(obs_output_t *output);
 
+	void ConnectSignals();
+
+	// Disconnects every handler ConnectSignals/InitOutput wired up and frees
+	// the CallbackData. Must run while m_output is still valid.
+	void DisconnectSignals();
+
+	static void OnStopped(void *data, calldata_t *params);
+
 	obs_video_info *m_canvas = nullptr;
 	obs_output_t *m_output = nullptr;
 
@@ -76,6 +91,7 @@ class Output {
 	bool m_outputStopped = false;
 
 	const std::vector<std::string> m_signals;
+	std::vector<CallbackData *> m_signalCallbackData;
 };
 
 }