From a7bbe9c4a3b2090240173e45bebab86e5cba3b4b Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 3 Mar 2015 16:17:51 +0100 Subject: [PATCH] Make sure Exceptions do not use unsafe format strings --- common/rdr/Exception.h | 6 ++--- common/rdr/FileInStream.cxx | 2 +- common/rdr/TLSException.cxx | 9 +------ common/rfb/CConnection.cxx | 9 +++---- common/rfb/Exception.h | 7 +++--- common/rfb/JpegCompressor.cxx | 4 +-- common/rfb/JpegDecompressor.cxx | 4 +-- vncviewer/parameters.cxx | 43 +++++++++++---------------------- 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 -- 2.39.5