AGS: Don't use unsafe strcat and strcpy

engines/ags/engine/ac/parser.cpp

@@ -103,19 +103,19 @@ int FindMatchingMultiWordWord(char *thisword, const char **text) {
 	const char *tempptr = *text;
 	char tempword[150] = "";
 	if (thisword != nullptr)
-		strcpy(tempword, thisword);
+		Common::strcpy_s(tempword, thisword);
 
 	int bestMatchFound = -1, word;
 	const char *tempptrAtBestMatch = tempptr;
 
 	do {
 		// extract and concat the next word
-		strcat(tempword, " ");
+		Common::strcat_s(tempword, " ");
 		while (tempptr[0] == ' ') tempptr++;
 		char chbuffer[2];
 		while (is_valid_word_char(tempptr[0])) {
 			snprintf(chbuffer, sizeof(chbuffer), "%c", tempptr[0]);
-			strcat(tempword, chbuffer);
+			Common::strcat_s(tempword, chbuffer);
 			tempptr++;
 		}
 		// is this it?
@@ -134,7 +134,7 @@ int FindMatchingMultiWordWord(char *thisword, const char **text) {
 		// yes, a word like "pick up" was found
 		*text = tempptrAtBestMatch;
 		if (thisword != nullptr)
-			strcpy(thisword, tempword);
+			Common::strcpy_s(thisword, 150, tempword);
 	}
 
 	return word;
@@ -247,7 +247,7 @@ int parse_sentence(const char *src_text, int *numwords, short *wordarray, short
 						continueSearching = 0;
 
 						if (text[0] == ' ') {
-							strcpy(thisword, textStart);
+							Common::strcpy_s(thisword, textStart);
 							thisword[text - textStart] = 0;
 							// forward past any multi-word alternatives
 							if (FindMatchingMultiWordWord(thisword, &text) >= 0)
@@ -275,7 +275,7 @@ int parse_sentence(const char *src_text, int *numwords, short *wordarray, short
 				// if it's an unknown word, store it for use in messages like
 				// "you can't use the word 'xxx' in this game"
 				if ((word < 0) && (_GP(play).bad_parsed_word[0] == 0))
-					strcpy(_GP(play).bad_parsed_word, thisword);
+					Common::strcpy_s(_GP(play).bad_parsed_word, 100, thisword);
 			}
 
 			if (do_word_now) {

engines/ags/engine/ac/string.cpp

@@ -53,9 +53,10 @@ const char *String_Copy(const char *srcString) {
 }
 
 const char *String_Append(const char *thisString, const char *extrabit) {
-	char *buffer = (char *)malloc(strlen(thisString) + strlen(extrabit) + 1);
-	strcpy(buffer, thisString);
-	strcat(buffer, extrabit);
+	size_t ln = strlen(thisString) + strlen(extrabit) + 1;
+	char *buffer = (char *)malloc(ln);
+	Common::strcpy_s(buffer, ln, thisString);
+	Common::strcat_s(buffer, ln, extrabit);
 	return CreateNewScriptString(buffer, false);
 }
 
@@ -317,7 +318,7 @@ void check_strlen(char *ptt) {
 /*void GetLanguageString(int indxx,char*buffr) {
 VALIDATE_STRING(buffr);
 char*bptr=get_language_text(indxx);
-if (bptr==NULL) strcpy(buffr,"[language string error]");
+if (bptr==NULL) Common::strcpy_s(buffr, 200, "[language string error]");
 else strncpy(buffr,bptr,199);
 buffr[199]=0;
 }*/
@@ -325,11 +326,7 @@ buffr[199]=0;
 void my_strncpy(char *dest, const char *src, int len) {
 	// the normal strncpy pads out the string with zeros up to the
 	// max length -- we don't want that
-	if (strlen(src) >= (unsigned)len) {
-		strncpy(dest, src, len);
-		dest[len] = 0;
-	} else
-		strcpy(dest, src);
+	Common::strcpy_s(dest, len + 1, src);
 }
 
 //=============================================================================

engines/ags/engine/gui/gui_dialog.cpp

@@ -114,7 +114,7 @@ int loadgamedialog() {
 				else {
 					toret = _G(filenumbers)[cursel];
 					String path = get_save_game_path(toret);
-					strcpy(_G(bufTemp), path.GetCStr());
+					Common::strcpy_s(_G(bufTemp), path.GetCStr());
 					_G(lpTemp) = &_G(bufTemp)[0];
 				}
 			} else if (mes.id == ctrlcancel) {
@@ -135,9 +135,9 @@ int loadgamedialog() {
 
 int savegamedialog() {
 	char okbuttontext[50];
-	strcpy(okbuttontext, get_global_message(MSG_SAVEBUTTON));
+	Common::strcpy_s(okbuttontext, get_global_message(MSG_SAVEBUTTON));
 	char labeltext[200];
-	strcpy(labeltext, get_global_message(MSG_SAVEDIALOG));
+	Common::strcpy_s(labeltext, get_global_message(MSG_SAVEDIALOG));
 	const int wnd_width = 200;
 	const int wnd_height = 120;
 	const int boxleft = _G(myscrnwid) / 2 - wnd_width / 2;
@@ -155,8 +155,8 @@ int savegamedialog() {
 	CSCISendControlMessage(ctrllist, CLB_CLEAR, 0, 0);    // clear the list box
 	preparesavegamelist(ctrllist);
 	if (_G(toomanygames)) {
-		strcpy(okbuttontext, get_global_message(MSG_REPLACE));
-		strcpy(labeltext, get_global_message(MSG_MUSTREPLACE));
+		Common::strcpy_s(okbuttontext, get_global_message(MSG_REPLACE));
+		Common::strcpy_s(labeltext, get_global_message(MSG_MUSTREPLACE));
 		labeltop = 2;
 	} else
 		ctrltbox = CSCICreateControl(CNT_TEXTBOX, 10, 29, 120, 0, nullptr);
@@ -184,7 +184,7 @@ int savegamedialog() {
 				if (_G(numsaves) > 0)
 					CSCISendControlMessage(ctrllist, CLB_GETTEXT, cursell, &_G(bufTemp)[0]);
 				else
-					strcpy(_G(bufTemp), "_NOSAVEGAMENAME");
+					Common::strcpy_s(_G(bufTemp), "_NOSAVEGAMENAME");
 
 				if (_G(toomanygames)) {
 					int nwhand = CSCIDrawWindow(boxleft + 5, boxtop + 20, 190, 65);
@@ -234,7 +234,7 @@ int savegamedialog() {
 
 					toret = highestnum + 1;
 					String path = get_save_game_path(toret);
-					strcpy(_G(bufTemp), path.GetCStr());
+					Common::strcpy_s(_G(bufTemp), path.GetCStr());
 				} else {
 					toret = _G(filenumbers)[cursell];
 					_G(bufTemp)[0] = 0;
@@ -242,7 +242,7 @@ int savegamedialog() {
 
 				if (_G(bufTemp)[0] == 0) {
 					String path = get_save_game_path(toret);
-					strcpy(_G(bufTemp), path.GetCStr());
+					Common::strcpy_s(_G(bufTemp), path.GetCStr());
 				}
 
 				_G(lpTemp) = &_G(bufTemp)[0];
@@ -331,7 +331,12 @@ void enterstringwindow(const char *prompttext, char *stouse) {
 	if (wantCancel)
 		CSCIDeleteControl(ctrlcancel);
 	CSCIEraseWindow(handl);
-	strcpy(stouse, _G(buffer2));
+	/* FIXME: Function should take a length parameter
+	 * It is called with a 200 bytes buffer below
+	 * but also called with a STD_BUFFER_SIZE (3000) buffer
+	 * and undetermined size buffer in the API
+	 * Using STD_BUFFER_SIZE as we don't want to break too much stuff */
+	Common::strcpy_s(stouse, STD_BUFFER_SIZE, _G(buffer2));
 }
 
 int enternumberwindow(char *prompttext) {
@@ -345,7 +350,7 @@ int enternumberwindow(char *prompttext) {
 int roomSelectorWindow(int currentRoom, int numRooms,
 		const std::vector<int> &roomNumbers, const std::vector<String> &roomNames) {
 	char labeltext[200];
-	strcpy(labeltext, get_global_message(MSG_SAVEDIALOG));
+	Common::strcpy_s(labeltext, get_global_message(MSG_SAVEDIALOG));
 	const int wnd_width = 240;
 	const int wnd_height = 160;
 	const int boxleft = _G(myscrnwid) / 2 - wnd_width / 2;
@@ -451,8 +456,8 @@ int myscimessagebox(const char *lpprompt, char *btn1, char *btn2) {
 
 int quitdialog() {
 	char quitbut[50], playbut[50];
-	strcpy(quitbut, get_global_message(MSG_QUITBUTTON));
-	strcpy(playbut, get_global_message(MSG_PLAYBUTTON));
+	Common::strcpy_s(quitbut, get_global_message(MSG_QUITBUTTON));
+	Common::strcpy_s(playbut, get_global_message(MSG_PLAYBUTTON));
 	return myscimessagebox(get_global_message(MSG_QUITDIALOG), quitbut, playbut);
 }
 

engines/ags/engine/gui/my_listbox.cpp

@@ -126,8 +126,9 @@ int MyListBox::pressedon(int mousex, int mousey) {
 void MyListBox::additem(char *texx) {
 	if (items >= MAXLISTITEM)
 		quit("!CSCIUSER16: Too many items added to listbox");
-	itemnames[items] = (char *)malloc(strlen(texx) + 1);
-	strcpy(itemnames[items], texx);
+	size_t ln = strlen(texx) + 1;
+	itemnames[items] = (char *)malloc(ln);
+	Common::strcpy_s(itemnames[items], ln, texx);
 	items++;
 	needredraw = 1;
 }
@@ -148,14 +149,15 @@ int MyListBox::processmessage(int mcode, int wParam, NumberPtr lParam) {
 		if (topitem + numonscreen <= selected)
 			topitem = (selected + 1) - numonscreen;
 	} else if (mcode == CLB_GETTEXT)
-		strcpy((char *)lParam._ptr, itemnames[wParam]);
+		Common::strcpy_s((char *)lParam._ptr, 260, itemnames[wParam]);
 	else if (mcode == CLB_SETTEXT) {
 		if (wParam < items)
 			free(itemnames[wParam]);
 
 		char *newstri = (char *)lParam._ptr;
-		itemnames[wParam] = (char *)malloc(strlen(newstri) + 2);
-		strcpy(itemnames[wParam], newstri);
+		size_t ln = strlen(newstri) + 2;
+		itemnames[wParam] = (char *)malloc(ln);
+		Common::strcpy_s(itemnames[wParam], ln, newstri);
 
 	} else if (mcode == CTB_KEYPRESS) {
 		if ((wParam == eAGSKeyCodeDownArrow) && (selected < items - 1))

engines/ags/engine/gui/my_textbox.cpp

@@ -35,7 +35,7 @@ MyTextBox::MyTextBox(int xx, int yy, int wii, const char *tee) {
 	y = yy;
 	wid = wii;
 	if (tee != nullptr)
-		strcpy(text, tee);
+		Common::strcpy_s(text, tee);
 	else
 		text[0] = 0;
 
@@ -64,7 +64,7 @@ int MyTextBox::processmessage(int mcode, int wParam, NumberPtr lParam) {
 		snprintf(text, sizeof(text), "%s", (const char *)lParam._ptr);
 		needredraw = 1;
 	} else if (mcode == CTB_GETTEXT)
-		strcpy((char *)lParam._ptr, text); // FIXME! dangerous
+		Common::strcpy_s((char *)lParam._ptr, 260, text); // FIXME! dangerous
 	else if (mcode == CTB_KEYPRESS) {
 		// NOTE: this deprecated control does not support UTF-8
 		int key = wParam;

engines/ags/engine/main/game_run.cpp

@@ -368,9 +368,9 @@ bool run_service_key_controls(KeyInput &out_key) {
 		for (int ff = 0; ff < _GP(game).numcharacters; ff++) {
 			if (_GP(game).chars[ff].room != _G(displayed_room)) continue;
 			if (strlen(bigbuffer) > 430) {
-				strcat(bigbuffer, "and more...");
+				Common::strcat_s(bigbuffer, "and more...");
 				Display(bigbuffer);
-				strcpy(bigbuffer, "CHARACTERS IN THIS ROOM (cont'd):[");
+				Common::strcpy_s(bigbuffer, "CHARACTERS IN THIS ROOM (cont'd):[");
 			}
 			chd = ff;
 			sprintf(&bigbuffer[strlen(bigbuffer)],

engines/ags/engine/main/quit.cpp

@@ -177,7 +177,7 @@ void quit(const char *quitmsg) {
 
 void quit_free() {
 	if (strlen(_G(quit_message)) == 0)
-		strcpy(_G(quit_message), "|bye!");
+		Common::strcpy_s(_G(quit_message), "|bye!");
 
 	const char *quitmsg = _G(quit_message);
 

engines/ags/plugins/ags_plugin.cpp

@@ -19,6 +19,9 @@
  *
  */
 
+// For dangerous AGS API
+#define FORBIDDEN_SYMBOL_EXCEPTION_strcpy
+
 #include "ags/lib/allegro.h"
 #include "ags/lib/std/vector.h"
 #include "ags/shared/core/platform.h"

engines/ags/plugins/ags_sprite_font/variable_width_sprite_font.cpp

@@ -103,13 +103,15 @@ void VariableWidthSpriteFontRenderer::SetLineHeightAdjust(int fontNum, int LineH
 void VariableWidthSpriteFontRenderer::EnsureTextValidForFont(char *text, int fontNumber) {
 	VariableWidthFont *font = getFontFor(fontNumber);
 	Common::String s(text);
+	size_t ln = s.size();
 
 	for (int i = (int)s.size() - 1; i >= 0 ; i--) {
 		if (font->characters.count(s[i]) == 0) {
 			s.erase(i, 1);
 		}
 	}
-	text = strcpy(text, s.c_str());
+	// We never grow the text
+	Common::strcpy_s(text, ln + 1, s.c_str());
 
 }