update force_fit_string and bounds in calling functions (#7161)

Rewrite `force_fit_string` to be more robust and to remove the potential for memory access violations.  Also audit all of the length parameters in the calling functions - some of them were the buffer size, not the length.  This should fix the mysterious crash in the mission log screen.

Author: Goober5000

code/graphics/software/font.cpp

@@ -559,24 +559,45 @@ namespace font
 		font_initialized = false;
 	}
 
-	int force_fit_string(char *str, int max_str, int max_width, float scale)
+	int force_fit_string(char *str, size_t max_str_len, int max_width, float scale)
 	{
-		int w;
+		if (max_width <= 0 || max_str_len == 0) {
+			*str = 0;
+			return 0;
+		}
+
+		size_t len = strlen(str);
+		if (len > max_str_len) {
+			len = max_str_len;
+			str[len] = 0;
+		}
 
+		int w;
 		gr_get_string_size(&w, nullptr, str, scale);
 		if (w > max_width) {
-			if ((int)strlen(str) > max_str - 3) {
-				Assert(max_str >= 3);
-				str[max_str - 3] = 0;
+			constexpr char ellipsis_char = '.';
+			constexpr size_t ellipsis_len = 3;
+
+			// make sure an ellipsis will fit
+			if (len < ellipsis_len) {
+				*str = 0;
+				return 0;
 			}
 
-			strcpy(str + strlen(str) - 1, "...");
-			gr_get_string_size(&w, nullptr, str, scale);
-			while (w > max_width) {
-				Assert(strlen(str) >= 4);  // if this is hit, a bad max_width was passed in and the calling function needs fixing.
-				strcpy(str + strlen(str) - 4, "...");
-				gr_get_string_size(&w, nullptr, str, scale);
+			// replace the last few chars with an ellipsis
+			for (size_t i = 0; i < ellipsis_len; ++i) {
+				--len;
+				str[len] = ellipsis_char;
 			}
+
+			// move the ellipsis back until the whole string fits
+			while (len > 0 && w > max_width) {
+				--len;
+				str[len] = ellipsis_char;
+				gr_get_string_size(&w, nullptr, str, scale, len + ellipsis_len);
+			};
+
+			str[len + ellipsis_len] = 0;
 		}
 
 		return w;

code/graphics/software/font.h

@@ -27,11 +27,11 @@ namespace font
 	* Does this by dropping characters at the end of the string and adding '...' to the end.
 	*
 	* @param str		string to crop.  Modifies this string directly
-	* @param max_str	max characters allowed in str
+	* @param max_str_len	max characters allowed in str (not including \0)
 	* @param max_width number of pixels to limit string to (less than or equal to).
 	* @return			The width of the string
 	*/
-	int force_fit_string(char *str, int max_str, int max_width, float scale = 1.0f);
+	int force_fit_string(char *str, size_t max_str_len, int max_width, float scale = 1.0f);
 
 	/**
 	* @brief Inites the font system

code/hud/hudescort.cpp

@@ -382,7 +382,7 @@ void HudGaugeEscort::renderIcon(int x, int y, int index, float scale, bool confi
 	// print out ship name
 	// original behavior replaced with similar logic to hudtargetbox.cpp, except
 	// if the name is hidden, it's replaced with the class name.
-	char buf[255];
+	char buf[256];
 	if (!config) {
 		if (((Iff_info[sp->team].flags & IFFF_WING_NAME_HIDDEN) && (sp->wingnum != -1)) ||
 			(sp->flags[Ship::Ship_Flags::Hide_ship_name])) {
@@ -474,7 +474,7 @@ void HudGaugeEscort::renderIcon(int x, int y, int index, float scale, bool confi
 void HudGaugeEscort::renderIconDogfight(int x, int y, int index)
 {
 	int			hull_integrity = 100;
-	char			buf[255];	
+	char			buf[256];
 	int			np_index;
 	object		*objp;
 

code/hud/hudsquadmsg.cpp

@@ -2934,7 +2934,7 @@ void HudGaugeSquadMessage::render(float  /*frametime*/, bool config)
 	for (int i = 0; i < nitems; i++ ) {
 		int item_num;
 		bool isSelectedItem = (i == Selected_menu_item);
-		char text[255];
+		char text[256];
 
 		if (!config) {
 			strcpy_s(text, MsgItems[First_menu_item + i].text.c_str());

code/hud/hudtarget.cpp

@@ -6092,7 +6092,7 @@ void HudGaugeWeaponEnergy::render(float /*frametime*/, bool config)
 			}
 			if(gr_screen.max_w_unscaled == 640) {
 				strcpy_s(shortened_name, weapon_name.c_str());
-				font::force_fit_string(shortened_name, NAME_LENGTH, fl2i(55 * scale), scale);
+				font::force_fit_string(shortened_name, NAME_LENGTH-1, fl2i(55 * scale), scale);
 				renderString(currentx, currenty, shortened_name, scale, config);
 			} else {
 				renderString(currentx, currenty, weapon_name.c_str(), scale, config);

code/menuui/barracks.cpp

@@ -38,8 +38,8 @@
 
 // stats defines
 //#define NUM_STAT_LINES (21 + MAX_SHIP_CLASSES)	// Goober5000
-#define STAT_COLUMN1_W 40*2							// as we might use Unicode //ksotar
-#define STAT_COLUMN2_W 10
+#define STAT_COLUMN1_W 41*2							// as we might use Unicode //ksotar
+#define STAT_COLUMN2_W 11
 
 static int Stat_column1_w[GR_NUM_RESOLUTIONS] =
 {

code/mission/missionlog.cpp

@@ -794,7 +794,7 @@ void mission_log_scrollback(int line_offset, int list_x, int list_y, int list_w,
 
 		char buf[256];
 		strcpy_s(buf, Log_scrollback_vec[i].objective.text.get());
-		font::force_fit_string(buf, 256, ACTION_X - OBJECT_X - 8);
+		font::force_fit_string(buf, 255, ACTION_X - OBJECT_X - 8);
 		gr_string(list_x + Log_scrollback_vec[i].objective.x, list_y + y, buf, GR_RESIZE_MENU);
 
 		// print the segments
@@ -814,7 +814,7 @@ void mission_log_scrollback(int line_offset, int list_x, int list_y, int list_w,
 			gr_set_color_fast(this_color);
 
 			strcpy_s(buf, thisSeg.text.get());
-			font::force_fit_string(buf, 256, list_w - thisSeg.x);
+			font::force_fit_string(buf, 255, list_w - thisSeg.x);
 			gr_string(list_x + thisSeg.x, list_y + seg_y, buf, GR_RESIZE_MENU);
 		}
 

code/missionui/missionbrief.cpp

@@ -1251,7 +1251,7 @@ void brief_render(float frametime)
 
 	if (Game_mode & GM_MULTIPLAYER) {
 		char buf[256];
-		strncpy(buf, The_mission.name, 256);
+		strncpy(buf, The_mission.name, 255);
 		font::force_fit_string(buf, 255, Title_coords_multi[gr_screen.res][2]);
 		gr_string(Title_coords_multi[gr_screen.res][0], Title_coords_multi[gr_screen.res][1], buf, GR_RESIZE_MENU);
 	} else {

code/network/multiui.cpp

@@ -1342,7 +1342,7 @@ void multi_join_display_games()
 			} 
 
 			// make sure the string fits in the display area and draw it
-			font::force_fit_string(str,200,Mj_game_name_coords[gr_screen.res][MJ_W_COORD]);			
+			font::force_fit_string(str, 199, Mj_game_name_coords[gr_screen.res][MJ_W_COORD]);
 			gr_string(Mj_game_name_coords[gr_screen.res][MJ_X_COORD],y_start,str,GR_RESIZE_MENU);
 
 			// display the ping time
@@ -4635,7 +4635,7 @@ void multi_create_list_do()
 	int idx;
 	int start_index,stop_index;
 	int line_height = gr_get_font_height() + 1;
-	char selected_name[255];
+	char selected_name[256];
 
 	// bail early if there aren't any selectable items
 	if(Multi_create_list_count == 0){
@@ -8357,7 +8357,7 @@ void multi_sync_post_close(bool API_Access)
 
 void multi_sync_display_name(const char *name,int index,int np_index)
 {
-	char fit[CALLSIGN_LEN];	
+	char fit[CALLSIGN_LEN+1];
 	int line_height = gr_get_font_height() + 1;
 
 	// make sure the string actually fits
@@ -8433,7 +8433,7 @@ void multi_sync_display_status(const char *status,int index)
 
 	// make sure the string actually fits
 	strcpy_s(fit, status);
-	font::force_fit_string(fit, 250, Ms_status2_coords[gr_screen.res][MS_W_COORD] - 20);
+	font::force_fit_string(fit, 249, Ms_status2_coords[gr_screen.res][MS_W_COORD] - 20);
 	gr_set_color_fast(&Color_bright);	
 	gr_string(Ms_status2_coords[gr_screen.res][MS_X_COORD], Ms_status2_coords[gr_screen.res][MS_Y_COORD] + (index * (gr_get_font_height() + 1)), fit, GR_RESIZE_MENU);		
 }

test/src/graphics/test_font.cpp

@@ -39,3 +39,45 @@ TEST_F(FontTest, additional_font_ttf) {
 
 	font::close();
 }
+
+TEST_F(FontTest, force_fit)
+{
+	font::init();
+
+	{
+		char str[] = "";
+		int w = font::force_fit_string(str, std::string::npos, 80);
+		ASSERT_EQ(w, 0);
+		ASSERT_EQ(strlen(str), 0);
+	}
+
+	{
+		char str[] = "abcdefghijklmnopqrstuvwxyz";
+		int w = font::force_fit_string(str, std::string::npos, 0);
+		ASSERT_EQ(w, 0);
+		ASSERT_EQ(strlen(str), 0);
+	}
+
+	{
+		char str[] = "abcdefghijklmnopqrstuvwxyz";
+		int w = font::force_fit_string(str, std::string::npos, 80);
+		ASSERT_LE(w, 80);
+		ASSERT_EQ(strlen(str), 12);
+	}
+
+	{
+		char str[] = "abcdefghijklmnopqrstuvwxyz";
+		int w = font::force_fit_string(str, 5, 300);
+		ASSERT_LE(w, 300);
+		ASSERT_EQ(strlen(str), 5);
+	}
+
+	{
+		char str[] = "abcdefghijklmnopqrstuvwxyz";
+		int w = font::force_fit_string(str, std::string::npos, 300);
+		ASSERT_LE(w, 300);
+		ASSERT_EQ(strlen(str), 26);
+	}
+
+	font::close();
+}