Avoid unsafe strcpy, strncpy, and strcat replacing with safer functions which

guaranty termination where possible.

14 files changed, 98 insertions, 75 deletions

diff --git a/cocoa/PlatCocoa.mm b/cocoa/PlatCocoa.mm
index 3d3417b82..5567088d1 100644
--- a/cocoa/PlatCocoa.mm
+++ b/cocoa/PlatCocoa.mm
@@ -1839,8 +1839,7 @@ void ListBoxImpl::GetValue(int n, char* value, int len)
     value[0] = '\0';
     return;
   }
-  strncpy(value, textString, len);
-  value[len - 1] = '\0';
+  strlcpy(value, textString, len);
 }
 
 void ListBoxImpl::RegisterImage(int type, const char* xpm_data)
@@ -2217,8 +2216,7 @@ bool Platform::ShowAssertionPopUps(bool assertionPopUps_)
 void Platform::Assert(const char *c, const char *file, int line)
 {
   char buffer[2000];
-  sprintf(buffer, "Assertion [%s] failed at %s %d", c, file, line);
-  strcat(buffer, "\r\n");
+  snprintf(buffer, sizeof(buffer), "Assertion [%s] failed at %s %d\r\n", c, file, line);
   Platform::DebugDisplay(buffer);
 #ifdef DEBUG
   // Jump into debugger in assert on Mac (CL269835)
diff --git a/gtk/Converter.h b/gtk/Converter.h
index fe9e23199..be530341f 100644
--- a/gtk/Converter.h
+++ b/gtk/Converter.h
@@ -51,8 +51,8 @@ public:
 			// Try allowing approximate transliterations
 			if (transliterations) {
 				char fullDest[200];
-				strcpy(fullDest, charSetDestination);
-				strcat(fullDest, "//TRANSLIT");
+				g_strlcpy(fullDest, charSetDestination, sizeof(fullDest));
+				g_strlcat(fullDest, "//TRANSLIT", sizeof(fullDest));
 				OpenHandle(fullDest, charSetSource);
 			}
 			if (!Succeeded()) {
diff --git a/gtk/PlatGTK.cxx b/gtk/PlatGTK.cxx
index da9171470..1479d50ee 100644
--- a/gtk/PlatGTK.cxx
+++ b/gtk/PlatGTK.cxx
@@ -23,8 +23,9 @@
 
 #include "Scintilla.h"
 #include "ScintillaWidget.h"
-#include "UniConversion.h"
+#include "StringCopy.h"
 #include "XPM.h"
+#include "UniConversion.h"
 
 #if defined(__clang__)
 // Clang 3.0 incorrectly displays  sentinel warnings. Fixed by clang 3.1.
@@ -231,7 +232,7 @@ static void SetLogFont(LOGFONT &lf, const char *faceName, int characterSet, floa
 	lf.weight = weight;
 	lf.italic = italic;
 	lf.characterSet = characterSet;
-	strncpy(lf.faceName, faceName, sizeof(lf.faceName) - 1);
+	StringCopy(lf.faceName, faceName);
 }
 
 /**
@@ -1830,8 +1831,7 @@ void ListBoxX::GetValue(int n, char *value, int len) {
 		gtk_tree_model_get(model, &iter, TEXT_COLUMN, &text, -1);
 	}
 	if (text && len > 0) {
-		strncpy(value, text, len);
-		value[len - 1] = '\0';
+		g_strlcpy(value, text, len);
 	} else {
 		value[0] = '\0';
 	}
@@ -2142,8 +2142,7 @@ bool Platform::ShowAssertionPopUps(bool assertionPopUps_) {
 
 void Platform::Assert(const char *c, const char *file, int line) {
 	char buffer[2000];
-	sprintf(buffer, "Assertion [%s] failed at %s %d", c, file, line);
-	strcat(buffer, "\r\n");
+	g_snprintf(buffer, sizeof(buffer), "Assertion [%s] failed at %s %d\r\n", c, file, line);
 	Platform::DebugDisplay(buffer);
 	abort();
 }
diff --git a/lexers/LexHTML.cxx b/lexers/LexHTML.cxx
index bb70fd45b..5ea24d481 100644
--- a/lexers/LexHTML.cxx
+++ b/lexers/LexHTML.cxx
@@ -16,6 +16,7 @@
 #include "Scintilla.h"
 #include "SciLexer.h"
 
+#include "StringCopy.h"
 #include "WordList.h"
 #include "LexAccessor.h"
 #include "Accessor.h"
@@ -929,9 +930,9 @@ static void ColouriseHyperTextDoc(unsigned int startPos, int length, int initSty
 															 (ch == '$' && chNext == '{') ||
 															 (ch == '<' && chNext == '/' && chNext2 == '%'))) {
 			if (ch == '%' || ch == '/')
-				strcpy(makoBlockType, "%");
+				StringCopy(makoBlockType, "%");
 			else if (ch == '$')
-				strcpy(makoBlockType, "{");
+				StringCopy(makoBlockType, "{");
 			else if (chNext == '/')
 				GetNextWord(styler, i+3, makoBlockType, sizeof(makoBlockType));
 			else
@@ -1000,9 +1001,9 @@ static void ColouriseHyperTextDoc(unsigned int startPos, int length, int initSty
 		// handle the start Django template code
 		else if (isDjango && scriptLanguage != eScriptPython && (ch == '{' && (chNext == '%' ||  chNext == '{'))) {
 			if (chNext == '%')
-				strcpy(djangoBlockType, "%");
+				StringCopy(djangoBlockType, "%");
 			else
-				strcpy(djangoBlockType, "{");
+				StringCopy(djangoBlockType, "{");
 			styler.ColourTo(i - 1, StateToPrint);
 			beforePreProc = state;
 			if (inScriptType == eNonHtmlScript)
@@ -1917,7 +1918,7 @@ static void ColouriseHyperTextDoc(unsigned int startPos, int length, int initSty
 					state = SCE_HPHP_COMMENTLINE;
 				} else if (ch == '\"') {
 					state = SCE_HPHP_HSTRING;
-					strcpy(phpStringDelimiter, "\"");
+					StringCopy(phpStringDelimiter, "\"");
 				} else if (styler.Match(i, "<<<")) {
 					bool isSimpleString = false;
 					i = FindPhpStringDelimiter(phpStringDelimiter, sizeof(phpStringDelimiter), i + 3, lengthDoc, styler, isSimpleString);
@@ -1927,7 +1928,7 @@ static void ColouriseHyperTextDoc(unsigned int startPos, int length, int initSty
 					}
 				} else if (ch == '\'') {
 					state = SCE_HPHP_SIMPLESTRING;
-					strcpy(phpStringDelimiter, "\'");
+					StringCopy(phpStringDelimiter, "\'");
 				} else if (ch == '$' && IsPhpWordStart(chNext)) {
 					state = SCE_HPHP_VARIABLE;
 				} else if (IsOperator(ch)) {
@@ -2047,7 +2048,7 @@ static void ColouriseHyperTextDoc(unsigned int startPos, int length, int initSty
 				state = SCE_HPHP_COMMENTLINE;
 			} else if (ch == '\"') {
 				state = SCE_HPHP_HSTRING;
-				strcpy(phpStringDelimiter, "\"");
+				StringCopy(phpStringDelimiter, "\"");
 			} else if (styler.Match(i, "<<<")) {
 				bool isSimpleString = false;
 				i = FindPhpStringDelimiter(phpStringDelimiter, sizeof(phpStringDelimiter), i + 3, lengthDoc, styler, isSimpleString);
@@ -2057,7 +2058,7 @@ static void ColouriseHyperTextDoc(unsigned int startPos, int length, int initSty
 				}
 			} else if (ch == '\'') {
 				state = SCE_HPHP_SIMPLESTRING;
-				strcpy(phpStringDelimiter, "\'");
+				StringCopy(phpStringDelimiter, "\'");
 			} else if (ch == '$' && IsPhpWordStart(chNext)) {
 				state = SCE_HPHP_VARIABLE;
 			} else if (IsOperator(ch)) {
diff --git a/lexlib/PropSetSimple.cxx b/lexlib/PropSetSimple.cxx
index 6792eea15..6f4553a07 100644
--- a/lexlib/PropSetSimple.cxx
+++ b/lexlib/PropSetSimple.cxx
@@ -146,7 +146,7 @@ int PropSetSimple::GetExpanded(const char *key, char *result) const {
 	ExpandAllInPlace(*this, val, 100, VarChain(key));
 	const int n = static_cast<int>(val.size());
 	if (result) {
-		strcpy(result, val.c_str());
+		memcpy(result, val.c_str(), n+1);
 	}
 	return n;	// Not including NUL
 }
diff --git a/lexlib/StringCopy.h b/lexlib/StringCopy.h
new file mode 100644
index 000000000..caca49911
--- /dev/null
+++ b/lexlib/StringCopy.h
@@ -0,0 +1,33 @@
+// Scintilla source code edit control
+/** @file StringCopy.h
+ ** Safe string copy function which always NUL terminates.
+ **/
+// Copyright 2013 by Neil Hodgson <neilh@scintilla.org>
+// The License.txt file describes the conditions under which this software may be distributed.
+
+#ifndef STRINGCOPY_H
+#define STRINGCOPY_H
+
+#ifdef SCI_NAMESPACE
+namespace Scintilla {
+#endif
+
+// Safer version of string copy functions like strcpy, wcsncpy, etc.
+// Instantiate over fixed length strings of both char and wchar_t.
+// May truncate if source doesn't fit into dest with room for NUL.
+
+template <typename T, size_t count>
+void StringCopy(T (&dest)[count], const T* source) {
+	for (size_t i=0; i<count; i++) {
+		dest[i] = source[i];
+		if (!source[i])
+			break;
+	}
+	dest[count-1] = 0;
+}
+
+#ifdef SCI_NAMESPACE
+}
+#endif
+
+#endif
diff --git a/lexlib/WordList.cxx b/lexlib/WordList.cxx
index 1f7127999..e789c0eaf 100644
--- a/lexlib/WordList.cxx
+++ b/lexlib/WordList.cxx
@@ -122,8 +122,9 @@ static void SortWordList(char **words, unsigned int len) {
 
 void WordList::Set(const char *s) {
 	Clear();
-	list = new char[strlen(s) + 1];
-	strcpy(list, s);
+	const size_t lenS = strlen(s) + 1;
+	list = new char[lenS];
+	memcpy(list, s, lenS);
 	words = ArrayFromWordList(list, &len, onlyLineEnds);
 #ifdef _MSC_VER
 	std::sort(words, words + len, cmpWords);
diff --git a/src/CaseConvert.cxx b/src/CaseConvert.cxx
index f983458c0..badaca411 100644
--- a/src/CaseConvert.cxx
+++ b/src/CaseConvert.cxx
@@ -13,6 +13,7 @@
 #include <vector>
 #include <algorithm>
 
+#include "StringCopy.h"
 #include "CaseConvert.h"
 #include "UniConversion.h"
 #include "UnicodeFromUTF8.h"
@@ -367,6 +368,9 @@ class CaseConverter : public ICaseConverter {
 	enum { maxConversionLength=6 };
 	struct ConversionString {
 		char conversion[maxConversionLength+1];
+		ConversionString() {
+			conversion[0] = '\0';
+		}
 	};
 	// Conversions are initially store in a vector of structs but then decomposed into
 	// parallel arrays as that is about 10% faster to search.
@@ -374,7 +378,7 @@ class CaseConverter : public ICaseConverter {
 		int character;
 		ConversionString conversion;
 		CharacterConversion(int character_=0, const char *conversion_="") : character(character_) {
-			strcpy(conversion.conversion, conversion_);
+			StringCopy(conversion.conversion, conversion_);
 		}
 		bool operator<(const CharacterConversion &other) const {
 			return character < other.character;
diff --git a/src/Editor.cxx b/src/Editor.cxx
index f254af4dd..da6b2eab3 100644
--- a/src/Editor.cxx
+++ b/src/Editor.cxx
@@ -7386,11 +7386,7 @@ sptr_t Editor::StyleGetMessage(unsigned int iMessage, uptr_t wParam, sptr_t lPar
 	case SCI_STYLEGETSIZEFRACTIONAL:
 		return vs.styles[wParam].size;
 	case SCI_STYLEGETFONT:
-		if (!vs.styles[wParam].fontName)
-			return 0;
-		if (lParam != 0)
-			strcpy(CharPtrFromSPtr(lParam), vs.styles[wParam].fontName);
-		return strlen(vs.styles[wParam].fontName);
+		return StringResult(lParam, vs.styles[wParam].fontName);
 	case SCI_STYLEGETUNDERLINE:
 		return vs.styles[wParam].underline ? 1 : 0;
 	case SCI_STYLEGETCASE:
@@ -7408,12 +7404,27 @@ sptr_t Editor::StyleGetMessage(unsigned int iMessage, uptr_t wParam, sptr_t lPar
 }
 
 sptr_t Editor::StringResult(sptr_t lParam, const char *val) {
-	const size_t n = strlen(val);
-	if (lParam != 0) {
-		char *ptr = reinterpret_cast<char *>(lParam);
-		strcpy(ptr, val);
+	const size_t len = val ? strlen(val) : 0;
+	if (lParam) {
+		char *ptr = CharPtrFromSPtr(lParam);
+		if (val)
+			memcpy(ptr, val, len+1);
+		else
+			*ptr = 0;
+	}
+	return len;	// Not including NUL
+}
+
+sptr_t Editor::BytesResult(sptr_t lParam, const unsigned char *val, size_t len) {
+	// No NUL termination: len is number of valid/displayed bytes
+	if (lParam) {
+		char *ptr = CharPtrFromSPtr(lParam);
+		if (val)
+			memcpy(ptr, val, len);
+		else
+			*ptr = 0;
 	}
-	return n;	// Not including NUL
+	return val ? len : 0;
 }
 
 sptr_t Editor::WndProc(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
@@ -9232,9 +9243,7 @@ sptr_t Editor::WndProc(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
 			Representation *repr = reprs.RepresentationFromCharacter(
 				reinterpret_cast<const char *>(wParam), UTF8MaxBytes);
 			if (repr) {
-				if (lParam != 0)
-					strcpy(CharPtrFromSPtr(lParam), repr->stringRep.c_str());
-				return repr->stringRep.size();
+				return StringResult(lParam, repr->stringRep.c_str());
 			}
 			return 0;
 		}
@@ -9350,13 +9359,7 @@ sptr_t Editor::WndProc(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
 
 	case SCI_MARGINGETTEXT: {
 			const StyledText st = pdoc->MarginStyledText(wParam);
-			if (lParam) {
-				if (st.text)
-					memcpy(CharPtrFromSPtr(lParam), st.text, st.length);
-				else
-					strcpy(CharPtrFromSPtr(lParam), "");
-			}
-			return st.length;
+			return BytesResult(lParam, reinterpret_cast<const unsigned char *>(st.text), st.length);
 		}
 
 	case SCI_MARGINSETSTYLE:
@@ -9374,13 +9377,7 @@ sptr_t Editor::WndProc(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
 
 	case SCI_MARGINGETSTYLES: {
 			const StyledText st = pdoc->MarginStyledText(wParam);
-			if (lParam) {
-				if (st.styles)
-					memcpy(CharPtrFromSPtr(lParam), st.styles, st.length);
-				else
-					strcpy(CharPtrFromSPtr(lParam), "");
-			}
-			return st.styles ? st.length : 0;
+			return BytesResult(lParam, st.styles, st.length);
 		}
 
 	case SCI_MARGINTEXTCLEARALL:
@@ -9393,13 +9390,7 @@ sptr_t Editor::WndProc(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
 
 	case SCI_ANNOTATIONGETTEXT: {
 			const StyledText st = pdoc->AnnotationStyledText(wParam);
-			if (lParam) {
-				if (st.text)
-					memcpy(CharPtrFromSPtr(lParam), st.text, st.length);
-				else
-					strcpy(CharPtrFromSPtr(lParam), "");
-			}
-			return st.length;
+			return BytesResult(lParam, reinterpret_cast<const unsigned char *>(st.text), st.length);
 		}
 
 	case SCI_ANNOTATIONGETSTYLE: {
@@ -9417,13 +9408,7 @@ sptr_t Editor::WndProc(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
 
 	case SCI_ANNOTATIONGETSTYLES: {
 			const StyledText st = pdoc->AnnotationStyledText(wParam);
-			if (lParam) {
-				if (st.styles)
-					memcpy(CharPtrFromSPtr(lParam), st.styles, st.length);
-				else
-					strcpy(CharPtrFromSPtr(lParam), "");
-			}
-			return st.styles ? st.length : 0;
+			return BytesResult(lParam, st.styles, st.length);
 		}
 
 	case SCI_ANNOTATIONGETLINES:
diff --git a/src/Editor.h b/src/Editor.h
index 7a8f1eb1d..c87708e92 100644
--- a/src/Editor.h
+++ b/src/Editor.h
@@ -632,6 +632,7 @@ protected:	// ScintillaBase subclass needs access to much of Editor
 	static const char *StringFromEOLMode(int eolMode);
 
 	static sptr_t StringResult(sptr_t lParam, const char *val);
+	static sptr_t BytesResult(sptr_t lParam, const unsigned char *val, size_t len);
 
 public:
 	// Public so the COM thunks can access it.
diff --git a/src/ScintillaBase.cxx b/src/ScintillaBase.cxx
index f8b989bd4..ad45a5ad2 100644
--- a/src/ScintillaBase.cxx
+++ b/src/ScintillaBase.cxx
@@ -381,7 +381,7 @@ int ScintillaBase::AutoCompleteGetCurrentText(char *buffer) const {
 		if (item != -1) {
 			const std::string selected = ac.GetValue(item);
 			if (buffer != NULL)
-				strcpy(buffer, selected.c_str());
+				memcpy(buffer, selected.c_str(), selected.length()+1);
 			return static_cast<int>(selected.length());
 		}
 	}
diff --git a/src/ViewStyle.cxx b/src/ViewStyle.cxx
index daa5b1a99..4b82c9c05 100644
--- a/src/ViewStyle.cxx
+++ b/src/ViewStyle.cxx
@@ -55,8 +55,9 @@ const char *FontNames::Save(const char *name) {
 			return *it;
 		}
 	}
-	char *nameSave = new char[strlen(name) + 1];
-	strcpy(nameSave, name);
+	const size_t lenName = strlen(name) + 1;
+	char *nameSave = new char[lenName];
+	memcpy(nameSave, name, lenName);
 	names.push_back(nameSave);
 	return nameSave;
 }
diff --git a/win32/PlatWin.cxx b/win32/PlatWin.cxx
index d2bc4c05f..90deb44e3 100644
--- a/win32/PlatWin.cxx
+++ b/win32/PlatWin.cxx
@@ -36,8 +36,9 @@
 #endif
 
 #include "Platform.h"
-#include "UniConversion.h"
+#include "StringCopy.h"
 #include "XPM.h"
+#include "UniConversion.h"
 #include "FontQuality.h"
 
 #ifndef IDC_HAND
@@ -307,8 +308,7 @@ static void SetLogFont(LOGFONTA &lf, const char *faceName, int characterSet, flo
 	lf.lfItalic = static_cast<BYTE>(italic ? 1 : 0);
 	lf.lfCharSet = static_cast<BYTE>(characterSet);
 	lf.lfQuality = Win32MapFontQuality(extraFontFlag);
-	strncpy(lf.lfFaceName, faceName, sizeof(lf.lfFaceName));
-	lf.lfFaceName[sizeof(lf.lfFaceName)-1] = '\0';
+	StringCopy(lf.lfFaceName, faceName);
 }
 
 /**
@@ -3214,7 +3214,7 @@ bool Platform::ShowAssertionPopUps(bool assertionPopUps_) {
 
 void Platform::Assert(const char *c, const char *file, int line) {
 	char buffer[2000];
-	sprintf(buffer, "Assertion [%s] failed at %s %d", c, file, line);
+	sprintf(buffer, "Assertion [%s] failed at %s %d%s", c, file, line, assertionPopUps ? "" : "\r\n");
 	if (assertionPopUps) {
 		int idButton = ::MessageBoxA(0, buffer, "Assertion failure",
 			MB_ABORTRETRYIGNORE|MB_ICONHAND|MB_SETFOREGROUND|MB_TASKMODAL);
@@ -3226,7 +3226,6 @@ void Platform::Assert(const char *c, const char *file, int line) {
 			abort();
 		}
 	} else {
-		strcat(buffer, "\r\n");
 		Platform::DebugDisplay(buffer);
 		::DebugBreak();
 		abort();
diff --git a/win32/ScintillaWin.cxx b/win32/ScintillaWin.cxx
index 2a6ac369d..8d5af5815 100644
--- a/win32/ScintillaWin.cxx
+++ b/win32/ScintillaWin.cxx
@@ -45,6 +45,7 @@
 #include "SciLexer.h"
 #include "LexerModule.h"
 #endif
+#include "StringCopy.h"
 #include "SplitVector.h"
 #include "Partitioning.h"
 #include "RunStyles.h"
@@ -2149,8 +2150,8 @@ void ScintillaWin::ImeStartComposition() {
 			lf.lfItalic = static_cast<BYTE>(vs.styles[styleHere].italic ? 1 : 0);
 			lf.lfCharSet = DEFAULT_CHARSET;
 			lf.lfFaceName[0] = '\0';
-			if (vs.styles[styleHere].fontName && (strlen(vs.styles[styleHere].fontName) < sizeof(lf.lfFaceName)))
-				strcpy(lf.lfFaceName, vs.styles[styleHere].fontName);
+			if (vs.styles[styleHere].fontName)
+				StringCopy(lf.lfFaceName, vs.styles[styleHere].fontName);
 
 			::ImmSetCompositionFontA(hIMC, &lf);
 		}