From 8a535fd5dd48315eaaf6b93d5c7534d0592addef Mon Sep 17 00:00:00 2001 From: BeaCox Date: Fri, 3 Jul 2026 21:12:49 +0800 Subject: [PATCH] fix: enforce clipboard receive size limits --- src/lib/client/Client.cpp | 5 + src/lib/client/Client.h | 1 + src/lib/client/ServerProxy.cpp | 17 +- src/lib/client/ServerProxy.h | 3 + src/lib/deskflow/ClipboardChunk.cpp | 87 ++++++- src/lib/deskflow/ClipboardChunk.h | 20 +- src/lib/server/ClientProxy1_6.cpp | 21 +- src/lib/server/ClientProxy1_6.h | 5 + src/lib/server/Server.cpp | 5 + src/lib/server/Server.h | 1 + .../deskflow/ClipboardChunksTests.cpp | 215 ++++++++++++++++++ src/unittests/deskflow/ClipboardChunksTests.h | 9 + 12 files changed, 358 insertions(+), 31 deletions(-) diff --git a/src/lib/client/Client.cpp b/src/lib/client/Client.cpp index b6fdebc19..935c380a1 100644 --- a/src/lib/client/Client.cpp +++ b/src/lib/client/Client.cpp @@ -175,6 +175,11 @@ NetworkAddress Client::getServerAddress() const return m_serverAddress; } +size_t Client::getMaximumClipboardSizeBytes() const +{ + return m_maximumClipboardSize * 1024; +} + void *Client::getEventTarget() const { return m_screen->getEventTarget(); diff --git a/src/lib/client/Client.h b/src/lib/client/Client.h index 4caf51d4e..b1954130c 100644 --- a/src/lib/client/Client.h +++ b/src/lib/client/Client.h @@ -126,6 +126,7 @@ public: { return m_resolvedAddressesCount; } + size_t getMaximumClipboardSizeBytes() const; //@} diff --git a/src/lib/client/ServerProxy.cpp b/src/lib/client/ServerProxy.cpp index 38df88634..555be346b 100644 --- a/src/lib/client/ServerProxy.cpp +++ b/src/lib/client/ServerProxy.cpp @@ -530,24 +530,29 @@ void ServerProxy::leave() void ServerProxy::setClipboard() { // parse - static std::string dataCached; ClipboardID id; uint32_t seq; - auto r = ClipboardChunk::assemble(m_stream, dataCached, id, seq); + auto r = ClipboardChunk::assemble( + m_stream, m_clipboardDataCached, id, seq, m_clipboardChunkState, m_client->getMaximumClipboardSizeBytes() + ); if (r == TransferState::Started) { - size_t size = ClipboardChunk::getExpectedSize(); - LOG_DEBUG("receiving clipboard %d size=%d", id, size); + size_t size = ClipboardChunk::getExpectedSize(m_clipboardChunkState); + LOG_DEBUG("receiving clipboard %d size=%zu", id, size); } else if (r == TransferState::Finished) { - LOG_DEBUG("received clipboard %d size=%d", id, dataCached.size()); + LOG_DEBUG("received clipboard %d size=%zu", id, m_clipboardDataCached.size()); // forward Clipboard clipboard; - clipboard.unmarshall(dataCached, 0); + clipboard.unmarshall(m_clipboardDataCached, 0); m_client->setClipboard(id, &clipboard); + m_clipboardDataCached.clear(); + m_clipboardDataCached.shrink_to_fit(); LOG_INFO("clipboard was updated"); + } else if (r == TransferState::Error) { + m_client->disconnect("invalid clipboard data from server"); } } diff --git a/src/lib/client/ServerProxy.h b/src/lib/client/ServerProxy.h index b53b6fedb..c6856a1d2 100644 --- a/src/lib/client/ServerProxy.h +++ b/src/lib/client/ServerProxy.h @@ -8,6 +8,7 @@ #pragma once +#include "deskflow/ClipboardChunk.h" #include "deskflow/ClipboardTypes.h" #include "deskflow/KeyTypes.h" #include "deskflow/KeyboardLayoutManager.h" @@ -124,6 +125,8 @@ private: MessageParser m_parser = &ServerProxy::parseHandshakeMessage; IEventQueue *m_events = nullptr; std::string m_serverLayout = ""; + std::string m_clipboardDataCached; + ClipboardChunkAssemblyState m_clipboardChunkState; bool m_isUserNotifiedAboutLayoutSyncError = false; deskflow::KeyboardLayoutManager m_layoutManager; }; diff --git a/src/lib/deskflow/ClipboardChunk.cpp b/src/lib/deskflow/ClipboardChunk.cpp index c0be88f49..7ee69a9c1 100644 --- a/src/lib/deskflow/ClipboardChunk.cpp +++ b/src/lib/deskflow/ClipboardChunk.cpp @@ -12,8 +12,22 @@ #include "deskflow/ProtocolUtil.h" #include "io/IStream.h" #include +#include -size_t ClipboardChunk::s_expectedSize = 0; +namespace { + +void clearCachedData(std::string &dataCached) +{ + dataCached.clear(); + dataCached.shrink_to_fit(); +} + +bool wouldExceed(size_t currentSize, size_t extraSize, size_t limit) +{ + return currentSize > limit || extraSize > limit - currentSize; +} + +} // namespace ClipboardChunk::ClipboardChunk(size_t size) : Chunk(size) { @@ -62,37 +76,88 @@ ClipboardChunk *ClipboardChunk::end(ClipboardID id, uint32_t sequence) return end; } -TransferState -ClipboardChunk::assemble(deskflow::IStream *stream, std::string &dataCached, ClipboardID &id, uint32_t &sequence) +TransferState ClipboardChunk::assemble( + deskflow::IStream *stream, std::string &dataCached, ClipboardID &id, uint32_t &sequence, + ClipboardChunkAssemblyState &state, size_t maxDataSize +) { using enum TransferState; uint8_t mark; std::string data; + auto reset = [&]() { + state = {}; + clearCachedData(dataCached); + }; if (!ProtocolUtil::readf(stream, kMsgDClipboard + 4, &id, &sequence, &mark, &data)) { + reset(); + return Error; + } + + if (id >= kClipboardEnd) { + LOG_ERR("clipboard chunk invalid id: %d", id); + reset(); return Error; } if (mark == ChunkType::DataStart) { - s_expectedSize = QString::fromStdString(data).toULong(); - LOG_DEBUG("start receiving clipboard data"); - dataCached.clear(); + bool ok = false; + const auto expected = QString::fromStdString(data).toULongLong(&ok); + if (!ok || expected > std::numeric_limits::max()) { + LOG_ERR("clipboard invalid size header: %s", data.c_str()); + reset(); + return Error; + } + + clearCachedData(dataCached); + state.expectedSize = static_cast(expected); + state.active = true; + + if (state.expectedSize > maxDataSize) { + LOG_ERR("clipboard size exceeds limit, size: %zu, limit: %zu", state.expectedSize, maxDataSize); + reset(); + return Error; + } + + LOG_DEBUG("start receiving clipboard data, expected size=%zu", state.expectedSize); return Started; } else if (mark == ChunkType::DataChunk) { + if (!state.active) { + LOG_ERR("clipboard data chunk before start"); + reset(); + return Error; + } + + if (wouldExceed(dataCached.size(), data.size(), state.expectedSize)) { + LOG_ERR( + "clipboard size exceeds declared, size: %zu, declared: %zu", dataCached.size() + data.size(), + state.expectedSize + ); + reset(); + return Error; + } + dataCached.append(data); return TransferState::InProgress; } else if (mark == ChunkType::DataEnd) { - // validate - if (id >= kClipboardEnd) { + if (!state.active) { + LOG_ERR("clipboard end chunk before start"); + reset(); return Error; - } else if (s_expectedSize != dataCached.size()) { - LOG_ERR("corrupted clipboard data, expected size=%d actual size=%d", s_expectedSize, dataCached.size()); + } + + state.active = false; + + if (state.expectedSize != dataCached.size()) { + LOG_ERR("corrupted clipboard data, expected size=%zu actual size=%zu", state.expectedSize, dataCached.size()); + reset(); return Error; } return Finished; } - LOG_ERR("clipboard transmission failed: unknown error"); + LOG_ERR("unknown clipboard chunk mark"); + reset(); return Error; } diff --git a/src/lib/deskflow/ClipboardChunk.h b/src/lib/deskflow/ClipboardChunk.h index dd98a5eb0..343e8b2fb 100644 --- a/src/lib/deskflow/ClipboardChunk.h +++ b/src/lib/deskflow/ClipboardChunk.h @@ -10,6 +10,7 @@ #include "deskflow/ClipboardTypes.h" #include "deskflow/ProtocolTypes.h" +#include #include constexpr static auto s_clipboardChunkMetaSize = 7; @@ -18,6 +19,12 @@ namespace deskflow { class IStream; } +struct ClipboardChunkAssemblyState +{ + size_t expectedSize = 0; + bool active = false; +}; + class ClipboardChunk : public Chunk { public: @@ -27,16 +34,15 @@ public: static ClipboardChunk *data(ClipboardID id, uint32_t sequence, const std::string &data); static ClipboardChunk *end(ClipboardID id, uint32_t sequence); - static TransferState - assemble(deskflow::IStream *stream, std::string &dataCached, ClipboardID &id, uint32_t &sequence); + static TransferState assemble( + deskflow::IStream *stream, std::string &dataCached, ClipboardID &id, uint32_t &sequence, + ClipboardChunkAssemblyState &state, size_t maxDataSize + ); static void send(deskflow::IStream *stream, void *data); - static size_t getExpectedSize() + static size_t getExpectedSize(const ClipboardChunkAssemblyState &state) { - return s_expectedSize; + return state.expectedSize; } - -private: - static size_t s_expectedSize; }; diff --git a/src/lib/server/ClientProxy1_6.cpp b/src/lib/server/ClientProxy1_6.cpp index 4334729d4..0c8d7d8b4 100644 --- a/src/lib/server/ClientProxy1_6.cpp +++ b/src/lib/server/ClientProxy1_6.cpp @@ -51,27 +51,34 @@ void ClientProxy1_6::setClipboard(ClipboardID id, const IClipboard *clipboard) bool ClientProxy1_6::recvClipboard() { // parse message - static std::string dataCached; ClipboardID id; uint32_t seq; - if (auto r = ClipboardChunk::assemble(getStream(), dataCached, id, seq); r == TransferState::Started) { - size_t size = ClipboardChunk::getExpectedSize(); - LOG_DEBUG("receiving clipboard %d size=%d", id, size); + auto r = ClipboardChunk::assemble( + getStream(), m_clipboardDataCached, id, seq, m_clipboardChunkState, m_server->getMaximumClipboardSizeBytes() + ); + + if (r == TransferState::Started) { + size_t size = ClipboardChunk::getExpectedSize(m_clipboardChunkState); + LOG_DEBUG("receiving clipboard %d size=%zu", id, size); } else if (r == TransferState::Finished) { LOG( - (CLOG_DEBUG "received client \"%s\" clipboard %d seqnum=%d, size=%d", getName().c_str(), id, seq, - dataCached.size()) + (CLOG_DEBUG "received client \"%s\" clipboard %d seqnum=%d, size=%zu", getName().c_str(), id, seq, + m_clipboardDataCached.size()) ); // save clipboard - m_clipboard[id].m_clipboard.unmarshall(dataCached, 0); + m_clipboard[id].m_clipboard.unmarshall(m_clipboardDataCached, 0); m_clipboard[id].m_sequenceNumber = seq; + m_clipboardDataCached.clear(); + m_clipboardDataCached.shrink_to_fit(); // notify auto *info = new ClipboardInfo; info->m_id = id; info->m_sequenceNumber = seq; m_events->addEvent(Event(EventTypes::ClipboardChanged, getEventTarget(), info)); + } else if (r == TransferState::Error) { + return false; } return true; diff --git a/src/lib/server/ClientProxy1_6.h b/src/lib/server/ClientProxy1_6.h index d66f0c5cf..22fe3c8f4 100644 --- a/src/lib/server/ClientProxy1_6.h +++ b/src/lib/server/ClientProxy1_6.h @@ -6,8 +6,11 @@ #pragma once +#include "deskflow/ClipboardChunk.h" #include "server/ClientProxy1_5.h" +#include + class Server; class IEventQueue; @@ -23,4 +26,6 @@ public: private: IEventQueue *m_events; + std::string m_clipboardDataCached; + ClipboardChunkAssemblyState m_clipboardChunkState; }; diff --git a/src/lib/server/Server.cpp b/src/lib/server/Server.cpp index 8ce755f24..f2b6617e1 100644 --- a/src/lib/server/Server.cpp +++ b/src/lib/server/Server.cpp @@ -183,6 +183,11 @@ Server::~Server() removeClient(m_primaryClient); } +size_t Server::getMaximumClipboardSizeBytes() const +{ + return m_maximumClipboardSize * 1024; +} + bool Server::setConfig(const ServerConfig &config) { // refuse configuration if it doesn't include the primary screen diff --git a/src/lib/server/Server.h b/src/lib/server/Server.h index 5c22b376b..4428e7a93 100644 --- a/src/lib/server/Server.h +++ b/src/lib/server/Server.h @@ -198,6 +198,7 @@ public: */ void getClients(std::vector &list) const; void sendConnectedClientsIpc() const; + size_t getMaximumClipboardSizeBytes() const; //@} diff --git a/src/unittests/deskflow/ClipboardChunksTests.cpp b/src/unittests/deskflow/ClipboardChunksTests.cpp index 33213e182..8fa0bd8b4 100644 --- a/src/unittests/deskflow/ClipboardChunksTests.cpp +++ b/src/unittests/deskflow/ClipboardChunksTests.cpp @@ -9,6 +9,162 @@ #include "deskflow/ClipboardChunk.h" #include "deskflow/ProtocolTypes.h" +#include "deskflow/ProtocolUtil.h" +#include "io/IStream.h" + +#include +#include +#include + +namespace { + +class MemoryStream : public deskflow::IStream +{ +public: + void push(const std::string &bytes) + { + m_queue.push_back(bytes); + } + + void close() override + { + m_queue.clear(); + m_inputShutdown = true; + } + + uint32_t read(void *buffer, uint32_t n) override + { + if (m_inputShutdown || m_queue.empty() || n == 0) { + return 0; + } + + auto &front = m_queue.front(); + const size_t take = std::min(static_cast(n), front.size()); + if (buffer != nullptr) { + std::memcpy(buffer, front.data(), take); + } + + front.erase(0, take); + if (front.empty()) { + m_queue.pop_front(); + } + + return static_cast(take); + } + + void write(const void *, uint32_t) override + { + } + + void flush() override + { + } + + void shutdownInput() override + { + close(); + } + + void shutdownOutput() override + { + } + + void *getEventTarget() const override + { + return const_cast(this); + } + + bool isReady() const override + { + return !m_inputShutdown && !m_queue.empty(); + } + + uint32_t getSize() const override + { + size_t total = 0; + for (const auto &chunk : m_queue) { + total += chunk.size(); + } + return static_cast(std::min(total, UINT32_MAX)); + } + +private: + std::deque m_queue; + bool m_inputShutdown = false; +}; + +class BufferWriteStream : public deskflow::IStream +{ +public: + const std::string &str() const + { + return m_buffer; + } + + void close() override + { + m_outputShutdown = true; + } + + uint32_t read(void *, uint32_t) override + { + return 0; + } + + void write(const void *buffer, uint32_t n) override + { + if (!m_outputShutdown && n != 0) { + m_buffer.append(static_cast(buffer), n); + } + } + + void flush() override + { + } + + void shutdownInput() override + { + } + + void shutdownOutput() override + { + m_outputShutdown = true; + } + + void *getEventTarget() const override + { + return const_cast(this); + } + + bool isReady() const override + { + return false; + } + + uint32_t getSize() const override + { + return 0; + } + +private: + std::string m_buffer; + bool m_outputShutdown = false; +}; + +std::string encodeClipboardMsg(ClipboardID id, uint32_t seq, uint8_t mark, const std::string &data) +{ + BufferWriteStream stream; + auto payload = data; + ProtocolUtil::writef(&stream, kMsgDClipboard + 4, id, seq, mark, &payload); + return stream.str(); +} + +} // namespace + +void ClipboardChunksTests::initTestCase() +{ + m_log.setFilter(LogLevel::Level::Debug); +} void ClipboardChunksTests::startFormatData() { @@ -70,4 +226,63 @@ void ClipboardChunksTests::endFormatData() delete chunk; } +void ClipboardChunksTests::assembleAllowsDataAtExpectedSizeAndLimit() +{ + MemoryStream stream; + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataStart, "4")); + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataChunk, "AB")); + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataChunk, "CD")); + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataEnd, "")); + + std::string cached; + ClipboardID id = kClipboardEnd; + uint32_t seq = 0; + ClipboardChunkAssemblyState state; + + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 4), TransferState::Started); + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 4), TransferState::InProgress); + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 4), TransferState::InProgress); + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 4), TransferState::Finished); + + QCOMPARE(cached, std::string("ABCD")); + QCOMPARE(id, static_cast(0)); + QCOMPARE(seq, static_cast(7)); + QCOMPARE(ClipboardChunk::getExpectedSize(state), static_cast(4)); + QVERIFY(!state.active); +} + +void ClipboardChunksTests::assembleRejectsDataBeyondExpectedSize() +{ + MemoryStream stream; + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataStart, "1")); + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataChunk, "AA")); + + std::string cached; + ClipboardID id = kClipboardEnd; + uint32_t seq = 0; + ClipboardChunkAssemblyState state; + + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 1024), TransferState::Started); + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 1024), TransferState::Error); + QVERIFY(cached.empty()); + QCOMPARE(ClipboardChunk::getExpectedSize(state), static_cast(0)); + QVERIFY(!state.active); +} + +void ClipboardChunksTests::assembleRejectsExpectedSizeBeyondLimit() +{ + MemoryStream stream; + stream.push(encodeClipboardMsg(0, 7, ChunkType::DataStart, "8")); + + std::string cached; + ClipboardID id = kClipboardEnd; + uint32_t seq = 0; + ClipboardChunkAssemblyState state; + + QCOMPARE(ClipboardChunk::assemble(&stream, cached, id, seq, state, 4), TransferState::Error); + QVERIFY(cached.empty()); + QCOMPARE(ClipboardChunk::getExpectedSize(state), static_cast(0)); + QVERIFY(!state.active); +} + QTEST_MAIN(ClipboardChunksTests) diff --git a/src/unittests/deskflow/ClipboardChunksTests.h b/src/unittests/deskflow/ClipboardChunksTests.h index 4f249c1cf..0c8436a86 100644 --- a/src/unittests/deskflow/ClipboardChunksTests.h +++ b/src/unittests/deskflow/ClipboardChunksTests.h @@ -4,6 +4,8 @@ * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception */ +#include "base/Log.h" + #include class ClipboardChunksTests : public QObject @@ -11,7 +13,14 @@ class ClipboardChunksTests : public QObject Q_OBJECT private Q_SLOTS: // Test are run in order top to bottom + void initTestCase(); void startFormatData(); void formatDataChunk(); void endFormatData(); + void assembleAllowsDataAtExpectedSizeAndLimit(); + void assembleRejectsDataBeyondExpectedSize(); + void assembleRejectsExpectedSizeBeyondLimit(); + +private: + Log m_log; };