Skip to content

Commit 295e077

Browse files
xusheng6xusheng6
authored
Parse remote protocol integers without throwing (#1166)
1 parent 4cf94e4 commit 295e077

5 files changed

Lines changed: 54 additions & 22 deletions

File tree

‎core/adapters/corelliumadapter.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ std::vector<DebugThread> CorelliumAdapter::GetThreadList()
353353
reply.AsString().substr(1);
354354
const auto tids = RspConnector::Split(shortened_string, ",");
355355
for ( const auto& tid : tids )
356-
threads.emplace_back(std::stoi(tid, nullptr, 16));
356+
threads.emplace_back(RspConnector::ParseInt<int>(tid));
357357

358358
reply = connector->TransmitAndReceive(RspData("qsThreadInfo"));
359359
}
@@ -872,7 +872,7 @@ DebugStopReason CorelliumAdapter::ResponseHandler()
872872
if (replyString.length() >= 3)
873873
{
874874
std::string signalString = replyString.substr(1, 2);
875-
uint64_t signal = std::stoull(signalString, nullptr, 16);
875+
uint64_t signal = RspConnector::ParseInt(signalString);
876876

877877
m_isTargetRunning = false;
878878

@@ -1042,7 +1042,7 @@ static std::string HexToAscii(const std::string& hex)
10421042
{
10431043
// Convert the two hex characters to a byte (using a stringstream)
10441044
std::string byte_string = hex.substr(i, 2);
1045-
unsigned char byte = static_cast<unsigned char>(std::stoi(byte_string, nullptr, 16)); // Convert to byte
1045+
unsigned char byte = static_cast<unsigned char>(RspConnector::ParseInt<int>(byte_string)); // Convert to byte
10461046

10471047
// Append the byte (ASCII char) to the resulting string
10481048
ascii.push_back(byte);

‎core/adapters/gdbadapter.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ std::vector<DebugThread> GdbAdapter::GetThreadList()
374374
reply.AsString().substr(1);
375375
const auto tids = RspConnector::Split(shortened_string, ",");
376376
for ( const auto& tid : tids )
377-
threads.emplace_back(std::stoi(tid, nullptr, 16));
377+
threads.emplace_back(RspConnector::ParseInt<int>(tid));
378378

379379
reply = connector->TransmitAndReceive(RspData("qsThreadInfo"));
380380
}
@@ -1013,8 +1013,8 @@ DebugStopReason GdbAdapter::ResponseHandler(bool notifyStopped)
10131013
if (replyString.length() >= 3)
10141014
{
10151015
std::string signalString = replyString.substr(1, 2);
1016-
uint64_t signal = std::stoull(signalString, nullptr, 16);
1017-
1016+
uint64_t signal = RspConnector::ParseInt(signalString);
1017+
10181018
m_isTargetRunning = false;
10191019
CheckApplyPendingBreakpoints();
10201020

@@ -1470,7 +1470,7 @@ static std::string HexToAscii(const std::string& hex)
14701470
{
14711471
// Convert the two hex characters to a byte (using a stringstream)
14721472
std::string byte_string = hex.substr(i, 2);
1473-
unsigned char byte = static_cast<unsigned char>(std::stoi(byte_string, nullptr, 16)); // Convert to byte
1473+
unsigned char byte = static_cast<unsigned char>(RspConnector::ParseInt<int>(byte_string)); // Convert to byte
14741474

14751475
// Append the byte (ASCII char) to the resulting string
14761476
ascii.push_back(byte);

‎core/adapters/gdbmiadapter.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "gdbmiadapter.h"
22
#include <sstream>
3+
#include <charconv>
34
#include <cinttypes>
45
#include "../debuggercontroller.h"
56
#include "../../cli/log.h"
@@ -953,7 +954,16 @@ DataBuffer GdbMiAdapter::ReadMemory(std::uintptr_t address, size_t size) {
953954
std::string hex_contents = value["memory"][0]["contents"].GetString();
954955
DataBuffer buffer(hex_contents.length() / 2);
955956
for(size_t i = 0; i < buffer.GetLength(); i++) {
956-
buffer[i] = std::stoul(hex_contents.substr(i*2, 2), nullptr, 16);
957+
// Parse with the non-throwing std::from_chars, since std::stoul throws on
958+
// malformed data coming from the gdb process
959+
unsigned int byte = 0;
960+
const char* first = hex_contents.data() + i * 2;
961+
if (std::from_chars(first, first + 2, byte, 16).ec != std::errc())
962+
{
963+
LogDebug("Malformed hex contents in memory read reply");
964+
return zero;
965+
}
966+
buffer[i] = byte;
957967
}
958968
return buffer;
959969
}

‎core/adapters/rspconnector.cpp‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,12 @@ RspData RspConnector::DecodeRLE(const RspData& data)
134134
std::unordered_map<std::string, std::uint64_t> RspConnector::PacketToUnorderedMap(const RspData& data)
135135
{
136136
std::unordered_map<std::string, std::uint64_t> packet_map{};
137-
packet_map["signal"] = std::stoull(data.AsString().substr(1, 2), nullptr, 16);
138-
139137
const auto data_string = data.AsString();
138+
if (data_string.length() < 3)
139+
return packet_map;
140+
141+
packet_map["signal"] = ParseInt(data_string.substr(1, 2));
142+
140143
const auto after_signal = data_string.substr(3);
141144

142145
for ( const auto& entries : RspConnector::Split(after_signal, ";")) {
@@ -156,17 +159,21 @@ std::unordered_map<std::string, std::uint64_t> RspConnector::PacketToUnorderedMa
156159
value = value.substr(0, 16);
157160

158161
if (key == "thread") {
159-
if (value[0] == 'p' && value.find('.') != std::string::npos) {
160-
auto core_id_and_thread_id = RspConnector::Split(value.substr(1), ".");
161-
packet_map["thread"] = std::stoull(core_id_and_thread_id[1], nullptr, 16);
162+
if (!value.empty() && value[0] == 'p' && value.find('.') != std::string::npos) {
163+
// Split takes a regex, so the separator has to be escaped -- an unescaped
164+
// "." matches every character and yields only empty tokens, which meant
165+
// multiprocess thread ids ("pPID.TID") never parsed.
166+
auto core_id_and_thread_id = RspConnector::Split(value.substr(1), "\\.");
167+
if (core_id_and_thread_id.size() >= 2)
168+
packet_map["thread"] = ParseInt(core_id_and_thread_id[1]);
162169
} else {
163-
packet_map["thread"] = std::stoull(value, nullptr, 16);
170+
packet_map["thread"] = ParseInt(value);
164171
}
165172
} else if (std::regex_search(key, std::regex("^[0-9a-fA-F]+$"))) {
166-
packet_map[fmt::format("r{}", std::stoi(key, nullptr, 16))] =
167-
static_cast<std::int64_t>( RspConnector::SwapEndianness(std::stoull(value, nullptr, 16)));
173+
packet_map[fmt::format("r{}", ParseInt<int>(key))] =
174+
static_cast<std::int64_t>( RspConnector::SwapEndianness(ParseInt(value)));
168175
} else {
169-
packet_map[key] = std::stoull(value, nullptr, 16);
176+
packet_map[key] = ParseInt(value);
170177
}
171178
}
172179
else
@@ -245,8 +252,8 @@ void RspConnector::NegotiateCapabilities(const std::vector <std::string>& capabi
245252
{
246253
if ( reply_token.find("PacketSize=") != std::string::npos )
247254
{
248-
if (auto packet_tokens = RspConnector::Split(reply_token, "="); !packet_tokens.empty())
249-
this->m_maxPacketLength = std::stoi(packet_tokens[1], nullptr, 16);
255+
if (auto packet_tokens = RspConnector::Split(reply_token, "="); packet_tokens.size() >= 2)
256+
this->m_maxPacketLength = ParseInt<int>(packet_tokens[1], 16, this->m_maxPacketLength);
250257
continue;
251258
}
252259

@@ -454,11 +461,13 @@ int32_t RspConnector::HostFileIO(const RspData& data, RspData& output, int32_t&
454461
if (resultErrno.find(',') != std::string::npos) {
455462
const auto split = RspConnector::Split(resultErrno, ",");
456463
if ((split.size() >= 2) && (split[1] != ""))
457-
error = std::stol(split[1].c_str(), nullptr, 16);
464+
error = ParseInt<int32_t>(split[1]);
458465

459-
return std::stol(split[0].c_str(), nullptr, 16);
466+
if (split.empty())
467+
return -1;
468+
return ParseInt<int32_t>(split[0], 16, -1);
460469
}
461-
return std::stol(resultErrno.c_str(), nullptr, 16);
470+
return ParseInt<int32_t>(resultErrno, 16, -1);
462471
}
463472

464473

‎core/adapters/rspconnector.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ limitations under the License.
2121
#include <vector>
2222
#include <unordered_map>
2323
#include <algorithm>
24+
#include <charconv>
2425
#include <regex>
2526
#include <array>
2627
#include "binaryninjaapi.h"
@@ -155,6 +156,18 @@ namespace BinaryNinjaDebugger
155156
static std::unordered_map<std::string, std::uint64_t> PacketToUnorderedMap(const RspData& data);
156157
static std::vector<std::string> Split(const std::string& string, const std::string& regex);
157158

159+
// Parse an integer from remote protocol data without throwing. std::stoi and friends
160+
// raise std::invalid_argument/std::out_of_range on malformed input, which crashes the
161+
// process when the string comes from an untrusted remote stub.
162+
template <typename Ty = uint64_t>
163+
static Ty ParseInt(const std::string& str, int base = 16, Ty fallback = 0)
164+
{
165+
Ty value = fallback;
166+
if (std::from_chars(str.data(), str.data() + str.size(), value, base).ec != std::errc())
167+
return fallback;
168+
return value;
169+
}
170+
158171
static uint64_t SwapEndianness(uint64_t value, size_t len)
159172
{
160173
switch (len)

0 commit comments

Comments
 (0)