From 7b9b1408231a9a5af7b3dd7e1a89c344d4add727 Mon Sep 17 00:00:00 2001 From: noita-player <56001276+noita-player@users.noreply.github.com> Date: Sun, 8 Mar 2026 20:00:50 -0700 Subject: [PATCH] Fix MCP use-after-free, scanner chunk overlap, build scripts - MCP bridge: guard against use-after-free when client disconnects during sendJson flush by re-checking m_client after write - Scanner engine: fix chunk overlap advancing past region end on final chunk; fix fallback region flags for providers without enumerateRegions - Build scripts: prefer GCC MinGW over LLVM-MinGW in PATH detection --- scripts/build.ps1 | 5 +++-- scripts/build_qscintilla.ps1 | 6 +++--- src/mcp/mcp_bridge.cpp | 22 +++++++++++++++++----- src/scanner.cpp | 11 +++++++---- tests/test_scanner_ui.cpp | 5 +++++ 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/scripts/build.ps1 b/scripts/build.ps1 index cd6872b..9861e84 100644 --- a/scripts/build.ps1 +++ b/scripts/build.ps1 @@ -283,9 +283,10 @@ function Find-MinGWDirectory { $toolsDir = Join-Path $qtRoot "Tools" if (Test-Path $toolsDir) { + # Prefer GCC-based MinGW (has g++.exe); exclude llvm-mingw. Prefer 64-bit, then newest. $mingwToolDirs = Get-ChildItem -Path $toolsDir -Directory -ErrorAction SilentlyContinue | Where-Object { - $_.Name -match 'mingw' - } + $_.Name -match '^mingw\d+_\d+$' + } | Sort-Object -Property @{ Expression = { if ($_.Name -match '_64$') { 1 } else { 0 } }; Descending = $true }, Name -Descending foreach ($dir in $mingwToolDirs) { $testBin = Join-Path $dir.FullName "bin\g++.exe" diff --git a/scripts/build_qscintilla.ps1 b/scripts/build_qscintilla.ps1 index 38dc4e3..f6efc18 100644 --- a/scripts/build_qscintilla.ps1 +++ b/scripts/build_qscintilla.ps1 @@ -318,10 +318,10 @@ $qtRoot = Split-Path (Split-Path $selectedQtDir -Parent) -Parent $toolsDir = Join-Path $qtRoot "Tools" if (Test-Path $toolsDir) { - # Look for MinGW tools directory + # Prefer GCC-based MinGW (has g++.exe); exclude llvm-mingw. Prefer 64-bit, then newest. $mingwToolDirs = Get-ChildItem -Path $toolsDir -Directory -ErrorAction SilentlyContinue | Where-Object { - $_.Name -match 'mingw' - } + $_.Name -match '^mingw\d+_\d+$' + } | Sort-Object -Property @{ Expression = { if ($_.Name -match '_64$') { 1 } else { 0 } }; Descending = $true }, Name -Descending foreach ($dir in $mingwToolDirs) { $testBin = Join-Path $dir.FullName "bin\g++.exe" diff --git a/src/mcp/mcp_bridge.cpp b/src/mcp/mcp_bridge.cpp index f8f0fc6..e22def0 100644 --- a/src/mcp/mcp_bridge.cpp +++ b/src/mcp/mcp_bridge.cpp @@ -45,9 +45,13 @@ void McpBridge::start() { void McpBridge::stop() { if (m_client) { + m_client->disconnect(this); m_client->disconnectFromServer(); + m_client->deleteLater(); m_client = nullptr; } + m_readBuffer.clear(); + m_initialized = false; if (m_server) { m_server->close(); delete m_server; @@ -65,8 +69,10 @@ void McpBridge::onNewConnection() { // Single client — disconnect previous if (m_client) { + m_client->disconnect(this); m_client->disconnectFromServer(); m_client->deleteLater(); + m_client = nullptr; } m_client = pending; @@ -82,10 +88,13 @@ void McpBridge::onNewConnection() { } void McpBridge::onReadyRead() { + if (!m_client) return; m_readBuffer.append(m_client->readAll()); // Newline-delimited JSON framing - while (true) { + // Guard: processLine→sendJson→flush can re-enter the event loop + // and trigger onDisconnected, nulling m_client mid-loop. + while (m_client) { int idx = m_readBuffer.indexOf('\n'); if (idx < 0) break; QByteArray line = m_readBuffer.left(idx).trimmed(); @@ -97,7 +106,12 @@ void McpBridge::onReadyRead() { void McpBridge::onDisconnected() { qDebug() << "[MCP] Client disconnected"; - m_client = nullptr; + if (m_client) { + m_client->disconnect(this); + m_client->deleteLater(); + m_client = nullptr; + } + m_readBuffer.clear(); m_initialized = false; } @@ -127,7 +141,7 @@ void McpBridge::sendJson(const QJsonObject& obj) { qDebug() << "[MCP] >>" << data.left(200); data.append('\n'); m_client->write(data); - m_client->flush(); + if (m_client) m_client->flush(); } void McpBridge::sendNotification(const QString& method, const QJsonObject& params) { @@ -172,12 +186,10 @@ void McpBridge::processLine(const QByteArray& line) { if (method == "initialize") { m_mainWindow->setMcpStatus(QStringLiteral("MCP: client connected")); - QCoreApplication::processEvents(); sendJson(handleInitialize(id, req.value("params").toObject())); m_mainWindow->clearMcpStatus(); } else if (method == "tools/list") { m_mainWindow->setMcpStatus(QStringLiteral("MCP: tools/list")); - QCoreApplication::processEvents(); sendJson(handleToolsList(id)); m_mainWindow->clearMcpStatus(); } else if (method == "tools/call") { diff --git a/src/scanner.cpp b/src/scanner.cpp index 950e0c3..03837cc 100644 --- a/src/scanner.cpp +++ b/src/scanner.cpp @@ -473,14 +473,14 @@ QVector ScanEngine::runScan(std::shared_ptr prov, << " filterExec:" << req.filterExecutable << " filterWrite:" << req.filterWritable; - // Fallback for providers that don't enumerate regions (file/buffer) + // Fallback for providers that don't enumerate regions (file/buffer/syscall without modules) if (regions.isEmpty()) { MemoryRegion fallback; fallback.base = 0; fallback.size = (uint64_t)prov->size(); fallback.readable = true; fallback.writable = true; - fallback.executable = false; + fallback.executable = true; // unknown; include so filters don't exclude the only region regions.append(fallback); } @@ -515,7 +515,8 @@ QVector ScanEngine::runScan(std::shared_ptr prov, constexpr int kChunk = 256 * 1024; - for (const auto& region : regions) { + for (int regionIndex = 0; regionIndex < regions.size(); ++regionIndex) { + const auto& region = regions[regionIndex]; if (m_abort.load()) break; if (req.filterExecutable && !region.executable) continue; @@ -540,7 +541,7 @@ QVector ScanEngine::runScan(std::shared_ptr prov, continue; } - const int overlap = patternLen - 1; + const int overlap = patternLen; // need full patternLen overlap so pattern at chunk end is found QByteArray chunk(qMin((uint64_t)kChunk, regSize), Qt::Uninitialized); uint64_t regOffset = regStart - region.base; // offset within provider region @@ -552,6 +553,8 @@ QVector ScanEngine::runScan(std::shared_ptr prov, if (!prov->read(regStart + off, chunk.data(), readLen)) { // Skip unreadable chunk + qDebug() << "[scan] read failed region" << regionIndex << "addr" << Qt::showbase << Qt::hex + << (region.base + off) << "base" << region.base << "off" << off << "len" << readLen << Qt::dec; off += readLen; scannedBytes += readLen; continue; diff --git a/tests/test_scanner_ui.cpp b/tests/test_scanner_ui.cpp index 638bec3..ebf4373 100644 --- a/tests/test_scanner_ui.cpp +++ b/tests/test_scanner_ui.cpp @@ -790,6 +790,7 @@ private slots: QByteArray newBytes(4, '\0'); std::memcpy(newBytes.data(), &newVal, 4); prov->writeBytes(8, newBytes); + m_panel->valueEdit()->setText("99"); // Click update — runs async QSignalSpy rescanSpy(m_panel->engine(), &ScanEngine::rescanFinished); @@ -839,6 +840,7 @@ private slots: std::memcpy(nb.data(), &newVal, 4); prov->writeBytes(i * 4, nb); } + m_panel->valueEdit()->setText("21"); // Click Re-scan — runs async QSignalSpy rescanSpy(m_panel->engine(), &ScanEngine::rescanFinished); @@ -930,6 +932,7 @@ private slots: QByteArray nb2(4, '\0'); std::memcpy(nb2.data(), &v2, 4); prov->writeBytes(4, nb2); + m_panel->valueEdit()->setText("20"); { QSignalSpy rescanSpy(m_panel->engine(), &ScanEngine::rescanFinished); QTest::mouseClick(m_panel->updateButton(), Qt::LeftButton); @@ -944,6 +947,7 @@ private slots: QByteArray nb3(4, '\0'); std::memcpy(nb3.data(), &v3, 4); prov->writeBytes(4, nb3); + m_panel->valueEdit()->setText("30"); { QSignalSpy rescanSpy(m_panel->engine(), &ScanEngine::rescanFinished); QTest::mouseClick(m_panel->updateButton(), Qt::LeftButton); @@ -1009,6 +1013,7 @@ private slots: int32_t newVal = kVal + iter; for (int off = 0; off + 4 <= kBufSize; off += kStride) std::memcpy(prov->data().data() + off, &newVal, 4); + m_panel->valueEdit()->setText(QString::number(newVal)); QElapsedTimer iterTimer; iterTimer.start();