Make sure Exceptions do not use unsafe format strings

8 files changed, 31 insertions, 53 deletions

diff --git a/common/rdr/Exception.h b/common/rdr/Exception.h
index f533dcce..62f452b2 100644
--- a/common/rdr/Exception.h
+++ b/common/rdr/Exception.h
@@ -43,15 +43,15 @@ namespace rdr {
   }; 
 
   struct TimedOut : public Exception {
-    TimedOut(const char* s="Timed out") : Exception(s) {}
+    TimedOut(const char* s="Timed out") : Exception("%s", s) {}
   };
  
   struct EndOfStream : public Exception {
-    EndOfStream(const char* s="End of stream") : Exception(s) {}
+    EndOfStream(const char* s="End of stream") : Exception("%s", s) {}
   };
 
   struct FrameException : public Exception {
-    FrameException(const char* s="Frame exception") : Exception(s) {}
+    FrameException(const char* s="Frame exception") : Exception("%s", s) {}
   };
 
 }
diff --git a/common/rdr/FileInStream.cxx b/common/rdr/FileInStream.cxx
index 18a9169d..6d23aa2c 100644
--- a/common/rdr/FileInStream.cxx
+++ b/common/rdr/FileInStream.cxx
@@ -72,7 +72,7 @@ int FileInStream::overrun(int itemSize, int nItems, bool wait)
     size_t n = fread((U8 *)end, b + sizeof(b) - end, 1, file);
     if (n < 1) {
       if (n < 0 || ferror(file))
-        throw Exception(strerror(errno));
+        throw SystemException("fread", errno);
       if (feof(file))
         throw EndOfStream();
       if (n == 0) { return 0; }
diff --git a/common/rdr/TLSException.cxx b/common/rdr/TLSException.cxx
index 3d39d790..bf8e97a2 100644
--- a/common/rdr/TLSException.cxx
+++ b/common/rdr/TLSException.cxx
@@ -34,15 +34,8 @@ using namespace rdr;
 
 #ifdef HAVE_GNUTLS
 TLSException::TLSException(const char* s, int err_)
-  : Exception(s), err(err_)
+  : Exception("%s: %s (%d)", s, gnutls_strerror(err), err), err(err_)
 {
-  strncat(str_, ": ", len-1-strlen(str_));
-  strncat(str_, gnutls_strerror(err), len-1-strlen(str_));
-  strncat(str_, " (", len-1-strlen(str_));
-  char buf[20];
-  sprintf(buf,"%d",err);
-  strncat(str_, buf, len-1-strlen(str_));
-  strncat(str_, ")", len-1-strlen(str_));
 }
 #endif /* HAVE_GNUTLS */
 
diff --git a/common/rfb/CConnection.cxx b/common/rfb/CConnection.cxx
index e0a23b5a..8ccd948b 100644
--- a/common/rfb/CConnection.cxx
+++ b/common/rfb/CConnection.cxx
@@ -97,12 +97,11 @@ void CConnection::processVersionMsg()
 
   // The only official RFB protocol versions are currently 3.3, 3.7 and 3.8
   if (cp.beforeVersion(3,3)) {
-    char msg[256];
-    sprintf(msg,"Server gave unsupported RFB protocol version %d.%d",
-            cp.majorVersion, cp.minorVersion);
-    vlog.error("%s", msg);
+    vlog.error("Server gave unsupported RFB protocol version %d.%d",
+               cp.majorVersion, cp.minorVersion);
     state_ = RFBSTATE_INVALID;
-    throw Exception(msg);
+    throw Exception("Server gave unsupported RFB protocol version %d.%d",
+                    cp.majorVersion, cp.minorVersion);
   } else if (useProtocol3_3 || cp.beforeVersion(3,7)) {
     cp.setVersion(3,3);
   } else if (cp.afterVersion(3,8)) {
diff --git a/common/rfb/Exception.h b/common/rfb/Exception.h
index 7c2cbcaa..5f47fcf2 100644
--- a/common/rfb/Exception.h
+++ b/common/rfb/Exception.h
@@ -24,14 +24,15 @@ namespace rfb {
   typedef rdr::Exception Exception;
   struct AuthFailureException : public Exception {
     AuthFailureException(const char* s="Authentication failure")
-      : Exception(s) {}
+      : Exception("%s", s) {}
   };
   struct AuthCancelledException : public rfb::Exception {
     AuthCancelledException(const char* s="Authentication cancelled")
-      : Exception(s) {}
+      : Exception("%s", s) {}
   };
   struct ConnFailedException : public Exception {
-    ConnFailedException(const char* s="Connection failed") : Exception(s) {}
+    ConnFailedException(const char* s="Connection failed")
+      : Exception("%s", s) {}
   };
 }
 #endif
diff --git a/common/rfb/JpegCompressor.cxx b/common/rfb/JpegCompressor.cxx
index c19af34e..5df0039e 100644
--- a/common/rfb/JpegCompressor.cxx
+++ b/common/rfb/JpegCompressor.cxx
@@ -123,7 +123,7 @@ JpegCompressor::JpegCompressor(int bufferLen) : MemOutStream(bufferLen)
 
   if(setjmp(err->jmpBuffer)) {
     // this will execute if libjpeg has an error
-    throw rdr::Exception(err->lastError);
+    throw rdr::Exception("%s", err->lastError);
   }
 
   jpeg_create_compress(cinfo);
@@ -166,7 +166,7 @@ void JpegCompressor::compress(const rdr::U8 *buf, int stride, const Rect& r,
     jpeg_abort_compress(cinfo);
     if (srcBufIsTemp && srcBuf) delete[] srcBuf;
     if (rowPointer) delete[] rowPointer;
-    throw rdr::Exception(err->lastError);
+    throw rdr::Exception("%s", err->lastError);
   }
 
   cinfo->image_width = w;
diff --git a/common/rfb/JpegDecompressor.cxx b/common/rfb/JpegDecompressor.cxx
index ca1ad226..70a4276f 100644
--- a/common/rfb/JpegDecompressor.cxx
+++ b/common/rfb/JpegDecompressor.cxx
@@ -116,7 +116,7 @@ JpegDecompressor::JpegDecompressor(void)
 
   if(setjmp(err->jmpBuffer)) {
     // this will execute if libjpeg has an error
-    throw rdr::Exception(err->lastError);
+    throw rdr::Exception("%s", err->lastError);
   }
 
   jpeg_create_decompress(dinfo);
@@ -162,7 +162,7 @@ void JpegDecompressor::decompress(const rdr::U8 *jpegBuf, int jpegBufLen,
     jpeg_abort_decompress(dinfo);
     if (dstBufIsTemp && dstBuf) delete[] dstBuf;
     if (rowPointer) delete[] rowPointer;
-    throw rdr::Exception(err->lastError);
+    throw rdr::Exception("%s", err->lastError);
   }
 
   src->pub.next_input_byte = jpegBuf;
diff --git a/vncviewer/parameters.cxx b/vncviewer/parameters.cxx
index db9e92d6..184af08f 100644
--- a/vncviewer/parameters.cxx
+++ b/vncviewer/parameters.cxx
@@ -489,7 +489,6 @@ void saveViewerParameters(const char *filename, const char *servername) {
 
   const size_t buffersize = 256;
   char filepath[PATH_MAX];
-  char write_error[buffersize*2];
   char encodingBuffer[buffersize];
 
   // Write to the registry or a predefined file if no filename was specified.
@@ -514,12 +513,9 @@ void saveViewerParameters(const char *filename, const char *servername) {
 
   /* Write parameters to file */
   FILE* f = fopen(filepath, "w+");
-  if (!f) {
-    snprintf(write_error, sizeof(write_error),
-             _("Failed to write configuration file, can't open %s: %s"),
-             filepath, strerror(errno));
-    throw Exception(write_error);
-  }
+  if (!f)
+    throw Exception(_("Failed to write configuration file, can't open %s: %s"),
+                    filepath, strerror(errno));
   
   fprintf(f, "%s\r\n", IDENTIFIER_STRING);
   fprintf(f, "\r\n");
@@ -548,7 +544,6 @@ char* loadViewerParameters(const char *filename) {
 
   const size_t buffersize = 256;
   char filepath[PATH_MAX];
-  char readError[buffersize*2];
   char line[buffersize];
   char decodingBuffer[buffersize];
   static char servername[sizeof(line)];
@@ -575,10 +570,8 @@ char* loadViewerParameters(const char *filename) {
   if (!f) {
     if (!filename)
       return NULL; // Use defaults.
-    snprintf(readError, sizeof(readError),
-             _("Failed to read configuration file, can't open %s: %s"),
-             filepath, strerror(errno));
-    throw Exception(readError);
+    throw Exception(_("Failed to read configuration file, can't open %s: %s"),
+                    filepath, strerror(errno));
   }
   
   int lineNr = 0;
@@ -587,31 +580,23 @@ char* loadViewerParameters(const char *filename) {
     // Read the next line
     lineNr++;
     if (!fgets(line, sizeof(line), f)) {
-      if (line[sizeof(line) -1] != '\0') {
-        snprintf(readError, sizeof(readError),
-                 _("Failed to read line %d in file %s: %s"),
-                 lineNr, filepath, _("Line too long"));
-        throw Exception(readError);
-      }
+      if (line[sizeof(line) -1] != '\0')
+        throw Exception(_("Failed to read line %d in file %s: %s"),
+                        lineNr, filepath, _("Line too long"));
       if (feof(f))
         break;
 
-      snprintf(readError, sizeof(readError),
-               _("Failed to read line %d in file %s: %s"),
-               lineNr, filepath, strerror(errno));
-      throw Exception(readError);
+      throw Exception(_("Failed to read line %d in file %s: %s"),
+                      lineNr, filepath, strerror(errno));
     }
     
     // Make sure that the first line of the file has the file identifier string
     if(lineNr == 1) {
-      if(strncmp(line, IDENTIFIER_STRING, strlen(IDENTIFIER_STRING)) == 0) {
+      if(strncmp(line, IDENTIFIER_STRING, strlen(IDENTIFIER_STRING)) == 0)
         continue;
-      } else {
-        snprintf(readError, sizeof(readError),
-                 _("Configuration file %s is in an invalid format"),
-                 filepath);
-        throw Exception(readError);
-      }
+      else
+        throw Exception(_("Configuration file %s is in an invalid format"),
+                        filepath);
     }
     
     // Skip empty lines and comments