Author: Igor Kudrin Date: 2025-03-19T10:51:27-07:00 New Revision: 825460a7728662d0062405e690485b7a1b689484
URL: https://github.com/llvm/llvm-project/commit/825460a7728662d0062405e690485b7a1b689484 DIFF: https://github.com/llvm/llvm-project/commit/825460a7728662d0062405e690485b7a1b689484.diff LOG: [lldb/gdb-remote] Do not crash on an invalid server response (#131979) An invalid RLE sequence in the received packet could result in an out-of-bounds reading that could cause a crash. Added: Modified: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index dad72a176b5fa..77eadfc8c9f6c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -788,9 +788,14 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len, // Copy the packet from m_bytes to packet_str expanding the run-length // encoding in the process. - std ::string packet_str = + auto maybe_packet_str = ExpandRLE(m_bytes.substr(content_start, content_end - content_start)); - packet = StringExtractorGDBRemote(packet_str); + if (!maybe_packet_str) { + m_bytes.erase(0, total_length); + packet.Clear(); + return GDBRemoteCommunication::PacketType::Invalid; + } + packet = StringExtractorGDBRemote(*maybe_packet_str); if (m_bytes[0] == '$' || m_bytes[0] == '%') { assert(checksum_idx < m_bytes.size()); @@ -1311,17 +1316,22 @@ void llvm::format_provider<GDBRemoteCommunication::PacketResult>::format( } } -std::string GDBRemoteCommunication::ExpandRLE(std::string packet) { +std::optional<std::string> +GDBRemoteCommunication::ExpandRLE(std::string packet) { // Reserve enough byte for the most common case (no RLE used). std::string decoded; decoded.reserve(packet.size()); for (std::string::const_iterator c = packet.begin(); c != packet.end(); ++c) { if (*c == '*') { + if (decoded.empty()) + return std::nullopt; // '*' indicates RLE. Next character will give us the repeat count and // previous character is what is to be repeated. char char_to_repeat = decoded.back(); // Number of time the previous character is repeated. - int repeat_count = *++c + 3 - ' '; + if (++c == packet.end()) + return std::nullopt; + int repeat_count = *c + 3 - ' '; // We have the char_to_repeat and repeat_count. Now push it in the // packet. for (int i = 0; i < repeat_count; ++i) @@ -1329,7 +1339,9 @@ std::string GDBRemoteCommunication::ExpandRLE(std::string packet) { } else if (*c == 0x7d) { // 0x7d is the escape character. The next character is to be XOR'd with // 0x20. - char escapee = *++c ^ 0x20; + if (++c == packet.end()) + return std::nullopt; + char escapee = *c ^ 0x20; decoded.push_back(escapee); } else { decoded.push_back(*c); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index 754797ba7f504..107c0896c4e61 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -168,7 +168,7 @@ class GDBRemoteCommunication : public Communication { GDBRemoteCommunication &server); /// Expand GDB run-length encoding. - static std::string ExpandRLE(std::string); + static std::optional<std::string> ExpandRLE(std::string); protected: std::chrono::seconds m_packet_timeout; diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp index 425f6cfcd5bc9..3da1f0a718fc5 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp @@ -68,3 +68,26 @@ TEST_F(GDBRemoteCommunicationTest, ReadPacket) { ASSERT_EQ(PacketResult::Success, server.GetAck()); } } + +// Test that packets with incorrect RLE sequences do not cause a crash and +// reported as invalid. +TEST_F(GDBRemoteCommunicationTest, CheckForPacket) { + using PacketType = GDBRemoteCommunication::PacketType; + struct TestCase { + llvm::StringLiteral Packet; + PacketType Result; + }; + static constexpr TestCase Tests[] = { + {{"$#00"}, PacketType::Standard}, + {{"$xx*#00"}, PacketType::Invalid}, // '*' without a count + {{"$*#00"}, PacketType::Invalid}, // '*' without a preceding character + {{"$xx}#00"}, PacketType::Invalid}, // bare escape character '}' + {{"%#00"}, PacketType::Notify}, // a correct packet after an invalid + }; + for (const auto &Test : Tests) { + SCOPED_TRACE(Test.Packet); + StringExtractorGDBRemote response; + EXPECT_EQ(Test.Result, client.CheckForPacket(Test.Packet.bytes_begin(), + Test.Packet.size(), response)); + } +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
