mirror of
https://github.com/Mudlet/Mudlet
synced 2026-08-13 18:26:27 -04:00
#### Brief overview of PR changes/additions * `cTelnet::processSocketData()` terminated its input at `in_buffer[amount + 1]`, one byte past the data it was given, and did so *before* checking the `-1`/`0` returns from `QIODevice::read()`. It now guards first, then terminates at `in_buffer[amount]`. * The guard is `amount <= 0` rather than `== -1`, because `loopbackTest()` narrows a `qsizetype` into an `int` and can produce a negative that is not `-1`. * Adds `cTelnetBufferTest` (5 slots) and drops the `reserve(size + 16)` slack that three existing telnet tests carried purely to absorb the stray write, which turns them into regression guards too. #### Motivation for adding to Mudlet The socket path survived this because `slot_socketReadyToBeRead()` over-allocates its stack buffer, but the same function is reached from Lua's `feedTelnet()` via `loopbackTest()`, which passes a `QByteArray` sized exactly to its contents - so the stray NUL landed one byte past a heap allocation. That is a real out-of-bounds write reachable from any script, and the workarounds already sitting in our test suite show it has been quietly worked around rather than fixed. #### Other info (issues closed, discussion etc) Closes #1065. Supersedes the closed #8438, which carried the same fix under 19 commits of unrelated history and had CI red on a faulty assertion in its own test. **Test case:** reverting the `src/ctelnet.cpp` hunk makes 3 of the 5 new slots fail and AddressSanitizer report `heap-buffer-overflow ... in cTelnet::processSocketData(char*, int, bool)`; restoring it gives 7 passed / 0 failed, and the full suite is 72/72 serially. One thing to flag for review: this adds `friend class cTelnetBufferTest;` to `cTelnet`, since `processSocketData()` is private and the public `loopbackTest()` cannot express a caller-laid-out buffer. It sits beside the existing `friend class TelnetTlsPromptTest;`, so there is precedent, but it is a test name in a shipped header and worth a second opinion. Three review findings were deliberately left out of scope. The MCCP decompression path hit the same overflow, via the re-entry that passes `remainingData`/`remainingAmount` back into `processSocketData()` - the fix covers it, but nothing under `test/` exercises compression at all, so it is fixed-but-unguarded and a dedicated MCCP test belongs in its own PR. The other two are pre-existing and orthogonal: a read error (`amount == -1`) is still silent, because `slot_socketError()` has been commented out as unused since 2017; and `mDecompressionRecursionDepth` is hand-balanced across four decrements rather than held by a scope guard (verified balanced today, but fragile). Happy to do any of them as a follow-up. Assisted-by: Claude:claude-opus-5
194 lines
7.5 KiB
C++
194 lines
7.5 KiB
C++
/***************************************************************************
|
|
* Copyright (C) 2026 by Mudlet Developers *
|
|
* *
|
|
* This program is free software; you can redistribute it and/or modify *
|
|
* it under the terms of the GNU General Public License as published by *
|
|
* the Free Software Foundation; either version 2 of the License, or *
|
|
* (at your option) any later version. *
|
|
* *
|
|
* This program is distributed in the hope that it will be useful, *
|
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of *
|
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
|
|
* GNU General Public License for more details. *
|
|
* *
|
|
* You should have received a copy of the GNU General Public License *
|
|
* along with this program; if not, write to the *
|
|
* Free Software Foundation, Inc., *
|
|
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
|
|
***************************************************************************/
|
|
|
|
#include <QDeadlineTimer>
|
|
#include <QtTest/QtTest>
|
|
|
|
#include <chrono>
|
|
|
|
#include "MudletInstanceCoordinator.h"
|
|
#include "TelnetServerStub.h"
|
|
#include "ctelnet.h"
|
|
#include "dlgConnectionProfiles.h"
|
|
#include "mudlet.h"
|
|
#include "utils.h"
|
|
|
|
extern void qInitResources_mudlet();
|
|
extern void qInitResources_qm();
|
|
extern void qInitResources_additional_splash_screens();
|
|
extern void qInitResources_mudlet_fonts_common();
|
|
extern void qInitResources_mudlet_fonts_posix();
|
|
void initializeQRCResourcesForSubnegotiation();
|
|
|
|
using namespace std::chrono_literals;
|
|
|
|
// Exercises recovery from a hostile/broken telnet subnegotiation: an IAC SB
|
|
// whose payload runs past the size cap without ever sending IAC SE. The rest of
|
|
// that subnegotiation must be dropped (not buffered without bound, and not
|
|
// leaked into the displayed stream) until the closing IAC SE, after which
|
|
// normal processing resumes.
|
|
class TelnetSubnegotiationTest : public QObject
|
|
{
|
|
Q_OBJECT
|
|
|
|
private:
|
|
TelnetServerStub* mpServer = nullptr;
|
|
const QString mHostname = "Test-Telnet-Subnegotiation";
|
|
const QString mPort = "4002";
|
|
const QString mLocalhost = "localhost";
|
|
|
|
private slots:
|
|
void initTestCase() { initializeQRCResourcesForSubnegotiation(); }
|
|
|
|
void init()
|
|
{
|
|
mpServer = new TelnetServerStub(qApp);
|
|
mpServer->start(mLocalhost, mPort.toUShort());
|
|
mudlet::start();
|
|
mudlet::self()->setupConfig();
|
|
mudlet::self()->takeOwnershipOfInstanceCoordinator(std::make_unique<MudletInstanceCoordinator>("MudletInstanceCoordinator"));
|
|
mudlet::self()->init();
|
|
mudlet::self()->setStorePasswordsSecurely(false);
|
|
deleteProfileDirectory(mHostname);
|
|
}
|
|
|
|
void test_oversizedSubnegotiationIsDroppedUntilSE()
|
|
{
|
|
startProfile(mHostname, mLocalhost, mPort);
|
|
auto host = mudlet::self()->getActiveHost();
|
|
QVERIFY2(host, "No active host available for the test.");
|
|
|
|
// MAX_TELNET_SUBNEGOTIATION_LENGTH in ctelnet.cpp is 5MB; send more
|
|
// than that inside the subnegotiation, with no IAC SE, then a marker
|
|
// that (only if recovery is broken) would leak into the display, then
|
|
// the real IAC SE and a line of ordinary text.
|
|
constexpr qsizetype overCapPadding = 5_MB + 1_KB;
|
|
QByteArray data;
|
|
data.append(TN_IAC);
|
|
data.append(TN_SB);
|
|
data.append(static_cast<char>(0x2d)); // an unused option; its value is irrelevant here
|
|
data.append(QByteArray(overCapPadding, 'A'));
|
|
data.append("SUBNEG_LEAK_MARKER");
|
|
data.append(TN_IAC);
|
|
data.append(TN_SE);
|
|
data.append("SUBNEG_RECOVERED\r\n");
|
|
host->mTelnet.loopbackTest(data);
|
|
|
|
QVERIFY2(waitForBufferToContain("SUBNEG_RECOVERED"), "Ordinary text after an oversized subnegotiation was not displayed - recovery failed.");
|
|
QVERIFY2(!bufferContains("SUBNEG_LEAK_MARKER"), "Subnegotiation payload past the size cap leaked into the display instead of being dropped until IAC SE.");
|
|
}
|
|
|
|
void cleanup()
|
|
{
|
|
delete mpServer;
|
|
mpServer = nullptr;
|
|
deleteProfileDirectory(mHostname);
|
|
delete mudlet::self();
|
|
}
|
|
|
|
// Utility function to manually start a profile like a user would do via the
|
|
// GUI
|
|
void startProfile(const QString& hostname, const QString& address, const QString& port)
|
|
{
|
|
QTimer::singleShot(0, qApp, [hostname, address, port]() {
|
|
mudlet::self()->startAutoLogin({});
|
|
QTest::qWait(100ms);
|
|
QTest::mouseClick(mudlet::self()->mpConnectionDialog->new_profile_button, Qt::LeftButton);
|
|
QTest::qWait(100ms);
|
|
QTest::keyClicks(QApplication::focusWidget(), hostname);
|
|
QTest::qWait(100ms);
|
|
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Tab);
|
|
QTest::qWait(100ms);
|
|
QTest::keyClicks(QApplication::focusWidget(), address);
|
|
QTest::qWait(100ms);
|
|
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Tab);
|
|
QTest::qWait(100ms);
|
|
QTest::keyClicks(QApplication::focusWidget(), port);
|
|
QTest::qWait(100ms);
|
|
QTest::keyClick(QApplication::focusWidget(), Qt::Key_Return);
|
|
});
|
|
|
|
QSignalSpy spy(mudlet::self(), &mudlet::signal_profileLoaded);
|
|
if (!spy.wait(5s)) {
|
|
QFAIL("Profile took too long to load.");
|
|
}
|
|
auto host = mudlet::self()->getActiveHost();
|
|
if (!host) {
|
|
QFAIL("No active host available for the test.");
|
|
}
|
|
|
|
QSignalSpy spy2(&(host->mTelnet), &cTelnet::signal_connected);
|
|
if (!spy2.wait(2s)) {
|
|
QFAIL("Could not connect with the host.");
|
|
}
|
|
}
|
|
|
|
// True if any line in the main console buffer contains the given substring
|
|
bool bufferContains(const QString& text)
|
|
{
|
|
auto console = mudlet::self()->getActiveHost()->mpConsole;
|
|
for (int i = 0; i <= console->buffer.getLastLineNumber(); ++i) {
|
|
if (console->buffer.line(i).contains(text)) {
|
|
return true;
|
|
}
|
|
}
|
|
return false;
|
|
}
|
|
|
|
// Polls the console buffer until the expected substring appears, with a timeout
|
|
bool waitForBufferToContain(const QString& text, std::chrono::milliseconds timeout = 5s)
|
|
{
|
|
return QTest::qWaitFor(
|
|
[&]() {
|
|
return bufferContains(text);
|
|
},
|
|
QDeadlineTimer(timeout));
|
|
}
|
|
|
|
// Utility function
|
|
void deleteProfileDirectory(const QString& profileName)
|
|
{
|
|
const QString path = mudlet::getMudletPath(enums::profileHomePath, profileName);
|
|
QDir dir(path);
|
|
|
|
if (!dir.exists()) {
|
|
qInfo() << "Profile directory does not exist:" << path;
|
|
return;
|
|
}
|
|
dir.removeRecursively();
|
|
}
|
|
};
|
|
|
|
void initializeQRCResourcesForSubnegotiation()
|
|
{
|
|
#ifdef INCLUDE_VARIABLE_SPLASH_SCREEN
|
|
qInitResources_additional_splash_screens();
|
|
#endif
|
|
#ifdef INCLUDE_FONTS
|
|
qInitResources_mudlet_fonts_common();
|
|
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
|
|
qInitResources_mudlet_fonts_posix();
|
|
#endif
|
|
#endif
|
|
qInitResources_mudlet();
|
|
qInitResources_qm();
|
|
}
|
|
|
|
#include "TelnetSubnegotiationTest.moc"
|
|
QTEST_MAIN(TelnetSubnegotiationTest)
|