Sanitize command strings before logging them.

gmaxwell · gmaxwell · commit 28d4cff0ed2d · 2015-02-08T19:58:59.000Z
Normally bitcoin core does not display any network originated strings without
 sanitizing or hex encoding.  This wasn't done for strcommand in many places.

This could be used to play havoc with a terminal displaying the logs,
 especially with printtoconsole in use.

Thanks to Evil-Knievel for reporting this issue.
diff --git a/src/main.cpp b/src/main.cpp
@@ -3450,7 +3450,7 @@ void static ProcessGetData(CNode* pfrom)
 bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, int64_t nTimeReceived)
 {
     RandAddSeedPerfmon();
-    LogPrint("net", "received: %s (%u bytes) peer=%d\n", strCommand, vRecv.size(), pfrom->id);
+    LogPrint("net", "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->id);
     if (mapArgs.count("-dropmessagestest") && GetRand(atoi(mapArgs["-dropmessagestest"])) == 0)
     {
         LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n");
@@ -4285,7 +4285,7 @@ bool ProcessMessages(CNode* pfrom)
 
         // Scan for message start
         if (memcmp(msg.hdr.pchMessageStart, Params().MessageStart(), MESSAGE_START_SIZE) != 0) {
-            LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", msg.hdr.GetCommand(), pfrom->id);
+            LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->id);
             fOk = false;
             break;
         }
@@ -4294,7 +4294,7 @@ bool ProcessMessages(CNode* pfrom)
         CMessageHeader& hdr = msg.hdr;
         if (!hdr.IsValid())
         {
-            LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", hdr.GetCommand(), pfrom->id);
+            LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->id);
             continue;
         }
         string strCommand = hdr.GetCommand();
@@ -4310,7 +4310,7 @@ bool ProcessMessages(CNode* pfrom)
         if (nChecksum != hdr.nChecksum)
         {
             LogPrintf("ProcessMessages(%s, %u bytes): CHECKSUM ERROR nChecksum=%08x hdr.nChecksum=%08x\n",
-               strCommand, nMessageSize, nChecksum, hdr.nChecksum);
+               SanitizeString(strCommand), nMessageSize, nChecksum, hdr.nChecksum);
             continue;
         }
 
@@ -4327,12 +4327,12 @@ bool ProcessMessages(CNode* pfrom)
             if (strstr(e.what(), "end of data"))
             {
                 // Allow exceptions from under-length message on vRecv
-                LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", strCommand, nMessageSize, e.what());
+                LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", SanitizeString(strCommand), nMessageSize, e.what());
             }
             else if (strstr(e.what(), "size too large"))
             {
                 // Allow exceptions from over-long size
-                LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught\n", strCommand, nMessageSize, e.what());
+                LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught\n", SanitizeString(strCommand), nMessageSize, e.what());
             }
             else
             {
@@ -4349,7 +4349,7 @@ bool ProcessMessages(CNode* pfrom)
         }
 
         if (!fRet)
-            LogPrintf("ProcessMessage(%s, %u bytes) FAILED peer=%d\n", strCommand, nMessageSize, pfrom->id);
+            LogPrintf("ProcessMessage(%s, %u bytes) FAILED peer=%d\n", SanitizeString(strCommand), nMessageSize, pfrom->id);
 
         break;
     }
diff --git a/src/net.cpp b/src/net.cpp
@@ -1971,7 +1971,7 @@ void CNode::BeginMessage(const char* pszCommand) EXCLUSIVE_LOCK_FUNCTION(cs_vSen
     ENTER_CRITICAL_SECTION(cs_vSend);
     assert(ssSend.size() == 0);
     ssSend << CMessageHeader(pszCommand, 0);
-    LogPrint("net", "sending: %s ", pszCommand);
+    LogPrint("net", "sending: %s ", SanitizeString(pszCommand));
 }
 
 void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend)
diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp
@@ -849,7 +849,7 @@ void JSONRequest::parse(const Value& valRequest)
         throw JSONRPCError(RPC_INVALID_REQUEST, "Method must be a string");
     strMethod = valMethod.get_str();
     if (strMethod != "getblocktemplate")
-        LogPrint("rpc", "ThreadRPCServer method=%s\n", strMethod);
+        LogPrint("rpc", "ThreadRPCServer method=%s\n", SanitizeString(strMethod));
 
     // Parse params
     Value valParams = find_value(request, "params");

Original file line number	Diff line number	Diff line change
`@@ -3450,7 +3450,7 @@ void static ProcessGetData(CNode* pfrom)`
`3450`	`3450`	`bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, int64_t nTimeReceived)`
`3451`	`3451`	`{`
`3452`	`3452`	`RandAddSeedPerfmon();`
`3453`		`- LogPrint("net", "received: %s (%u bytes) peer=%d\n", strCommand, vRecv.size(), pfrom->id);`
	`3453`	`+ LogPrint("net", "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->id);`
`3454`	`3454`	`if (mapArgs.count("-dropmessagestest") && GetRand(atoi(mapArgs["-dropmessagestest"])) == 0)`
`3455`	`3455`	`{`
`3456`	`3456`	`LogPrintf("dropmessagestest DROPPING RECV MESSAGE\n");`
`@@ -4285,7 +4285,7 @@ bool ProcessMessages(CNode* pfrom)`
`4285`	`4285`
`4286`	`4286`	`// Scan for message start`
`4287`	`4287`	`if (memcmp(msg.hdr.pchMessageStart, Params().MessageStart(), MESSAGE_START_SIZE) != 0) {`
`4288`		`- LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", msg.hdr.GetCommand(), pfrom->id);`
	`4288`	`+ LogPrintf("PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->id);`
`4289`	`4289`	`fOk = false;`
`4290`	`4290`	`break;`
`4291`	`4291`	`}`
`@@ -4294,7 +4294,7 @@ bool ProcessMessages(CNode* pfrom)`
`4294`	`4294`	`CMessageHeader& hdr = msg.hdr;`
`4295`	`4295`	`if (!hdr.IsValid())`
`4296`	`4296`	`{`
`4297`		`- LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", hdr.GetCommand(), pfrom->id);`
	`4297`	`+ LogPrintf("PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->id);`
`4298`	`4298`	`continue;`
`4299`	`4299`	`}`
`4300`	`4300`	`string strCommand = hdr.GetCommand();`
`@@ -4310,7 +4310,7 @@ bool ProcessMessages(CNode* pfrom)`
`4310`	`4310`	`if (nChecksum != hdr.nChecksum)`
`4311`	`4311`	`{`
`4312`	`4312`	`LogPrintf("ProcessMessages(%s, %u bytes): CHECKSUM ERROR nChecksum=%08x hdr.nChecksum=%08x\n",`
`4313`		`- strCommand, nMessageSize, nChecksum, hdr.nChecksum);`
	`4313`	`+ SanitizeString(strCommand), nMessageSize, nChecksum, hdr.nChecksum);`
`4314`	`4314`	`continue;`
`4315`	`4315`	`}`
`4316`	`4316`
`@@ -4327,12 +4327,12 @@ bool ProcessMessages(CNode* pfrom)`
`4327`	`4327`	`if (strstr(e.what(), "end of data"))`
`4328`	`4328`	`{`
`4329`	`4329`	`// Allow exceptions from under-length message on vRecv`
`4330`		`- LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", strCommand, nMessageSize, e.what());`
	`4330`	`+ LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", SanitizeString(strCommand), nMessageSize, e.what());`
`4331`	`4331`	`}`
`4332`	`4332`	`else if (strstr(e.what(), "size too large"))`
`4333`	`4333`	`{`
`4334`	`4334`	`// Allow exceptions from over-long size`
`4335`		`- LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught\n", strCommand, nMessageSize, e.what());`
	`4335`	`+ LogPrintf("ProcessMessages(%s, %u bytes): Exception '%s' caught\n", SanitizeString(strCommand), nMessageSize, e.what());`
`4336`	`4336`	`}`
`4337`	`4337`	`else`
`4338`	`4338`	`{`
`@@ -4349,7 +4349,7 @@ bool ProcessMessages(CNode* pfrom)`
`4349`	`4349`	`}`
`4350`	`4350`
`4351`	`4351`	`if (!fRet)`
`4352`		`- LogPrintf("ProcessMessage(%s, %u bytes) FAILED peer=%d\n", strCommand, nMessageSize, pfrom->id);`
	`4352`	`+ LogPrintf("ProcessMessage(%s, %u bytes) FAILED peer=%d\n", SanitizeString(strCommand), nMessageSize, pfrom->id);`
`4353`	`4353`
`4354`	`4354`	`break;`
`4355`	`4355`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1971,7 +1971,7 @@ void CNode::BeginMessage(const char* pszCommand) EXCLUSIVE_LOCK_FUNCTION(cs_vSen`
`1971`	`1971`	`ENTER_CRITICAL_SECTION(cs_vSend);`
`1972`	`1972`	`assert(ssSend.size() == 0);`
`1973`	`1973`	`ssSend << CMessageHeader(pszCommand, 0);`
`1974`		`- LogPrint("net", "sending: %s ", pszCommand);`
	`1974`	`+ LogPrint("net", "sending: %s ", SanitizeString(pszCommand));`
`1975`	`1975`	`}`
`1976`	`1976`
`1977`	`1977`	`void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend)`