Clean up string encoding handling

We should handle this in the low-level protocol code as much as possible
to avoid mistakes. This way the rest of the code can assume that strings
are always UTF-8 with \n line endings.

8 files changed, 86 insertions, 23 deletions

diff --git a/common/rfb/CConnection.cxx b/common/rfb/CConnection.cxx
index c07df216..a043f9bd 100644
--- a/common/rfb/CConnection.cxx
+++ b/common/rfb/CConnection.cxx
@@ -543,7 +543,7 @@ void CConnection::serverCutText(const char* str)
 {
   hasLocalClipboard = false;
 
-  serverClipboard = latin1ToUTF8(str);
+  serverClipboard = str;
   hasRemoteClipboard = true;
 
   handleClipboardAnnounce(true);
@@ -604,6 +604,11 @@ void CConnection::handleClipboardProvide(uint32_t flags,
     return;
   }
 
+  // FIXME: This conversion magic should be in CMsgReader
+  if (!isValidUTF8((const char*)data[0], lengths[0])) {
+    vlog.error("Invalid UTF-8 sequence in clipboard - ignoring");
+    return;
+  }
   serverClipboard = convertLF((const char*)data[0], lengths[0]);
   hasRemoteClipboard = true;
 
@@ -674,6 +679,7 @@ void CConnection::announceClipboard(bool available)
 void CConnection::sendClipboardData(const char* data)
 {
   if (server.clipboardFlags() & rfb::clipboardProvide) {
+    // FIXME: This conversion magic should be in CMsgWriter
     std::string filtered(convertCRLF(data));
     size_t sizes[1] = { filtered.size() + 1 };
     const uint8_t* data[1] = { (const uint8_t*)filtered.c_str() };
@@ -690,9 +696,7 @@ void CConnection::sendClipboardData(const char* data)
 
     writer()->writeClipboardProvide(rfb::clipboardUTF8, sizes, data);
   } else {
-    std::string latin1(utf8ToLatin1(data));
-
-    writer()->writeClientCutText(latin1.c_str());
+    writer()->writeClientCutText(data);
   }
 }
 
diff --git a/common/rfb/CMsgReader.cxx b/common/rfb/CMsgReader.cxx
index c0a96690..006645df 100644
--- a/common/rfb/CMsgReader.cxx
+++ b/common/rfb/CMsgReader.cxx
@@ -76,7 +76,12 @@ bool CMsgReader::readServerInit()
   std::vector<char> name(len + 1);
   is->readBytes((uint8_t*)name.data(), len);
   name[len] = '\0';
-  handler->serverInit(width, height, pf, name.data());
+
+  if (isValidUTF8(name.data()))
+    handler->serverInit(width, height, pf, name.data());
+  else
+    handler->serverInit(width, height, pf,
+                        latin1ToUTF8(name.data()).c_str());
 
   return true;
 }
@@ -275,9 +280,13 @@ bool CMsgReader::readServerCutText()
     vlog.error("cut text too long (%d bytes) - ignoring",len);
     return true;
   }
+
   std::vector<char> ca(len);
   is->readBytes((uint8_t*)ca.data(), len);
-  std::string filtered(convertLF(ca.data(), len));
+
+  std::string utf8(latin1ToUTF8(ca.data(), ca.size()));
+  std::string filtered(convertLF(utf8.data(), utf8.size()));
+
   handler->serverCutText(filtered.c_str());
 
   return true;
@@ -768,10 +777,16 @@ bool CMsgReader::readSetDesktopName(int x, int y, int w, int h)
 
   if (x || y || w || h) {
     vlog.error("Ignoring DesktopName rect with non-zero position/size");
-  } else {
-    handler->setName(name.data());
+    return true;
   }
 
+  if (!isValidUTF8(name.data())) {
+    vlog.error("Ignoring DesktopName rect with invalid UTF-8 sequence");
+    return true;
+  }
+
+  handler->setName(name.data());
+
   return true;
 }
 
diff --git a/common/rfb/CMsgWriter.cxx b/common/rfb/CMsgWriter.cxx
index 0ff81926..e941aaa7 100644
--- a/common/rfb/CMsgWriter.cxx
+++ b/common/rfb/CMsgWriter.cxx
@@ -36,6 +36,7 @@
 #include <rfb/Rect.h>
 #include <rfb/ServerParams.h>
 #include <rfb/CMsgWriter.h>
+#include <rfb/util.h>
 
 using namespace rfb;
 
@@ -191,16 +192,15 @@ void CMsgWriter::writePointerEvent(const Point& pos, int buttonMask)
 
 void CMsgWriter::writeClientCutText(const char* str)
 {
-  size_t len;
-
   if (strchr(str, '\r') != NULL)
     throw Exception("Invalid carriage return in clipboard data");
 
-  len = strlen(str);
+  std::string latin1(utf8ToLatin1(str));
+
   startMsg(msgTypeClientCutText);
   os->pad(3);
-  os->writeU32(len);
-  os->writeBytes((const uint8_t*)str, len);
+  os->writeU32(latin1.size());
+  os->writeBytes((const uint8_t*)latin1.data(), latin1.size());
   endMsg();
 }
 
diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx
index 2b18d8c1..33b2d850 100644
--- a/common/rfb/SConnection.cxx
+++ b/common/rfb/SConnection.cxx
@@ -383,7 +383,7 @@ void SConnection::clientCutText(const char* str)
 {
   hasLocalClipboard = false;
 
-  clientClipboard = latin1ToUTF8(str);
+  clientClipboard = str;
   hasRemoteClipboard = true;
 
   handleClipboardAnnounce(true);
@@ -429,6 +429,11 @@ void SConnection::handleClipboardProvide(uint32_t flags,
     return;
   }
 
+  // FIXME: This conversion magic should be in SMsgReader
+  if (!isValidUTF8((const char*)data[0], lengths[0])) {
+    vlog.error("Invalid UTF-8 sequence in clipboard - ignoring");
+    return;
+  }
   clientClipboard = convertLF((const char*)data[0], lengths[0]);
   hasRemoteClipboard = true;
 
@@ -592,6 +597,7 @@ void SConnection::sendClipboardData(const char* data)
 {
   if (client.supportsEncoding(pseudoEncodingExtendedClipboard) &&
       (client.clipboardFlags() & rfb::clipboardProvide)) {
+    // FIXME: This conversion magic should be in SMsgWriter
     std::string filtered(convertCRLF(data));
     size_t sizes[1] = { filtered.size() + 1 };
     const uint8_t* data[1] = { (const uint8_t*)filtered.c_str() };
@@ -608,9 +614,7 @@ void SConnection::sendClipboardData(const char* data)
 
     writer()->writeClipboardProvide(rfb::clipboardUTF8, sizes, data);
   } else {
-    std::string latin1(utf8ToLatin1(data));
-
-    writer()->writeServerCutText(latin1.c_str());
+    writer()->writeServerCutText(data);
   }
 }
 
diff --git a/common/rfb/SMsgReader.cxx b/common/rfb/SMsgReader.cxx
index 68c9365b..0792639a 100644
--- a/common/rfb/SMsgReader.cxx
+++ b/common/rfb/SMsgReader.cxx
@@ -316,7 +316,10 @@ bool SMsgReader::readClientCutText()
 
   std::vector<char> ca(len);
   is->readBytes((uint8_t*)ca.data(), len);
-  std::string filtered(convertLF(ca.data(), len));
+
+  std::string utf8(latin1ToUTF8(ca.data(), ca.size()));
+  std::string filtered(convertLF(utf8.data(), utf8.size()));
+
   handler->clientCutText(filtered.c_str());
 
   return true;
diff --git a/common/rfb/SMsgWriter.cxx b/common/rfb/SMsgWriter.cxx
index 95f85352..8592e6f4 100644
--- a/common/rfb/SMsgWriter.cxx
+++ b/common/rfb/SMsgWriter.cxx
@@ -38,6 +38,7 @@
 #include <rfb/SMsgWriter.h>
 #include <rfb/LogWriter.h>
 #include <rfb/ledStates.h>
+#include <rfb/util.h>
 
 using namespace rfb;
 
@@ -92,16 +93,15 @@ void SMsgWriter::writeBell()
 
 void SMsgWriter::writeServerCutText(const char* str)
 {
-  size_t len;
-
   if (strchr(str, '\r') != NULL)
     throw Exception("Invalid carriage return in clipboard data");
 
-  len = strlen(str);
+  std::string latin1(utf8ToLatin1(str));
+
   startMsg(msgTypeServerCutText);
   os->pad(3);
-  os->writeU32(len);
-  os->writeBytes((const uint8_t*)str, len);
+  os->writeU32(latin1.size());
+  os->writeBytes((const uint8_t*)latin1.data(), latin1.size());
   endMsg();
 }
 
diff --git a/common/rfb/util.cxx b/common/rfb/util.cxx
index c5c00bbd..a3f16443 100644
--- a/common/rfb/util.cxx
+++ b/common/rfb/util.cxx
@@ -564,6 +564,40 @@ namespace rfb {
     return out;
   }
 
+  bool isValidUTF8(const char* str, size_t bytes)
+  {
+    while ((bytes > 0) && (*str != '\0')) {
+      size_t len;
+      unsigned ucs;
+
+      len = utf8ToUCS4(str, bytes, &ucs);
+      str += len;
+      bytes -= len;
+
+      if (ucs == 0xfffd)
+        return false;
+    }
+
+    return true;
+  }
+
+  bool isValidUTF16(const wchar_t* wstr, size_t units)
+  {
+    while ((units > 0) && (*wstr != '\0')) {
+      size_t len;
+      unsigned ucs;
+
+      len = utf16ToUCS4(wstr, units, &ucs);
+      wstr += len;
+      units -= len;
+
+      if (ucs == 0xfffd)
+        return false;
+    }
+
+    return true;
+  }
+
   unsigned msBetween(const struct timeval *first,
                      const struct timeval *second)
   {
diff --git a/common/rfb/util.h b/common/rfb/util.h
index 34811e3f..cafea209 100644
--- a/common/rfb/util.h
+++ b/common/rfb/util.h
@@ -68,6 +68,9 @@ namespace rfb {
   std::string utf16ToUTF8(const wchar_t* src, size_t units = (size_t)-1);
   std::wstring utf8ToUTF16(const char* src, size_t bytes = (size_t)-1);
 
+  bool isValidUTF8(const char* str, size_t bytes = (size_t)-1);
+  bool isValidUTF16(const wchar_t* wstr, size_t units = (size_t)-1);
+
   // HELPER functions for timeout handling
 
   // soonestTimeout() is a function to help work out the soonest of several