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.
This commit is contained in:
baldurk
2023-05-19 10:28:58 +01:00
parent e0464fea4f
commit 1f72a09e3b
5 changed files with 47 additions and 12 deletions
+11
View File
@@ -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)
+1 -1
View File
@@ -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);
}
+18 -11
View File
@@ -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;
+12
View File
@@ -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())
+5
View File
@@ -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);