File 0003-Sanitise-strings-printed-when-received-from-target-c.patch of Package renderdoc.18053
From 12eba72c4df34bb8ad316c96957548ac5fc04310 Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Fri, 19 May 2023 10:28:58 +0100
Subject: [PATCH 3/5] Sanitise strings printed when received from target
control/remote server
* Given socket corruption or network errors these strings could contain
unprintable characters so we sanitise them reasonably. This also ameliorates a
potential security concern with arbitrary strings being written to a log, but
these connections are still considered trusted and users should not be
exposing RenderDoc ports to the internet.
---
renderdoc/common/common.cpp | 11 +++++++++++
renderdoc/core/remote_server.cpp | 2 +-
renderdoc/core/target_control.cpp | 29 ++++++++++++++++++-----------
renderdoc/strings/string_utils.cpp | 12 ++++++++++++
renderdoc/strings/string_utils.h | 5 +++++
5 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/renderdoc/common/common.cpp b/renderdoc/common/common.cpp
index f026eea16..670b3fbd0 100644
--- a/renderdoc/common/common.cpp
+++ b/renderdoc/common/common.cpp
@@ -473,6 +473,17 @@ void rdclog_direct(time_t utcTime, uint32_t pid, LogType type, const char *proje
va_end(args2);
}
+ // normalise newlines
+ {
+ char *nl = base;
+ while(*nl)
+ {
+ if(*nl == '\r')
+ *nl = '\n';
+ nl++;
+ }
+ }
+
// likely path - string contains no newlines
char *nl = strchr(base, '\n');
if(nl == NULL)
diff --git a/renderdoc/core/remote_server.cpp b/renderdoc/core/remote_server.cpp
index 4944ab394..5153562f2 100644
--- a/renderdoc/core/remote_server.cpp
+++ b/renderdoc/core/remote_server.cpp
@@ -464,7 +464,7 @@ static void ActiveRemoteClientThread(ClientThread *threadData,
reader.EndChunk();
- RDCLOG("Taking ownership of '%s'.", path.c_str());
+ RDCLOG("Taking ownership of capture.");
tempFiles.push_back(path);
}
diff --git a/renderdoc/core/target_control.cpp b/renderdoc/core/target_control.cpp
index a63a4a2e6..bfc6a3ddd 100644
--- a/renderdoc/core/target_control.cpp
+++ b/renderdoc/core/target_control.cpp
@@ -31,6 +31,7 @@
#include "os/os_specific.h"
#include "replay/replay_driver.h"
#include "serialise/serialiser.h"
+#include "strings/string_utils.h"
static const uint32_t TargetControlProtocolVersion = 9;
@@ -484,6 +485,8 @@ void RenderDoc::TargetControlServerThread(Network::Socket *sock)
ser.EndChunk();
+ strip_nonbasic(newClient);
+
if(newClient.empty() || !IsProtocolVersionSupported(version))
{
RDCLOG("Invalid/Unsupported handshake '%s' / %d", newClient.c_str(), version);
@@ -605,12 +608,23 @@ public:
m_Version = 0;
+ if(type == ePacket_Handshake)
{
READ_DATA_SCOPE();
SERIALISE_ELEMENT(m_Version);
SERIALISE_ELEMENT(m_Target);
SERIALISE_ELEMENT(m_PID);
}
+ else if(type == ePacket_Busy)
+ {
+ READ_DATA_SCOPE();
+ SERIALISE_ELEMENT(m_Version);
+ SERIALISE_ELEMENT(m_Target);
+ SERIALISE_ELEMENT(m_BusyClient);
+ }
+
+ strip_nonbasic(m_Target);
+ strip_nonbasic(m_BusyClient);
reader.EndChunk();
@@ -745,17 +759,6 @@ public:
reader.EndChunk();
return msg;
}
- else if(type == ePacket_Busy)
- {
- READ_DATA_SCOPE();
- SERIALISE_ELEMENT(msg.busy.clientName).Named("Client Name"_lit);
-
- SAFE_DELETE(m_Socket);
-
- RDCLOG("Got busy signal: '%s", msg.busy.clientName.c_str());
- msg.type = TargetControlMessageType::Busy;
- return msg;
- }
else if(type == ePacket_NewChild)
{
msg.type = TargetControlMessageType::NewChild;
@@ -884,8 +887,12 @@ public:
RDCLOG("Used API: %s (%s & %s)", msg.apiUse.name.c_str(),
presenting ? "Presenting" : "Not presenting",
supported ? "supported" : "not supported");
+
if(!supportMessage.empty())
+ {
+ strip_nonbasic(supportMessage);
RDCLOG("Support: %s", supportMessage.c_str());
+ }
reader.EndChunk();
return msg;
diff --git a/renderdoc/strings/string_utils.cpp b/renderdoc/strings/string_utils.cpp
index 100ec9773..b2d02c8b4 100644
--- a/renderdoc/strings/string_utils.cpp
+++ b/renderdoc/strings/string_utils.cpp
@@ -141,6 +141,18 @@ rdcstr strip_extension(const rdcstr &path)
return path.substr(0, offs);
}
+rdcstr strip_nonbasic(rdcstr &str)
+{
+ for(char &c : str)
+ {
+ if((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '.' ||
+ c == ' ')
+ continue;
+
+ c = '_';
+ }
+}
+
void split(const rdcstr &in, rdcarray<rdcstr> &out, const char sep)
{
if(in.empty())
diff --git a/renderdoc/strings/string_utils.h b/renderdoc/strings/string_utils.h
index e833b7263..bb6c45a2f 100644
--- a/renderdoc/strings/string_utils.h
+++ b/renderdoc/strings/string_utils.h
@@ -37,5 +37,10 @@ rdcstr get_basename(const rdcstr &path);
rdcstr get_dirname(const rdcstr &path);
rdcstr strip_extension(const rdcstr &path);
+// remove everything but alphanumeric ' ' and '.'
+// It replaces everything else with _
+// for logging strings where they might contain garbage characters
+rdcstr strip_nonbasic(rdcstr &str);
+
void split(const rdcstr &in, rdcarray<rdcstr> &out, const char sep);
void merge(const rdcarray<rdcstr> &in, rdcstr &out, const char sep);
--
2.41.0