(svn r3745) Fix two buffer overflows, one case of undefined behavior (the destination buffer of sprintf() may not alias with one of its arguments) and some other minor stuff introduced in r3740

1 files changed, 64 insertions, 53 deletions

diff --git a/os/macosx/macos.m b/os/macosx/macos.m
index 5ac2c4922..59801fce0 100644
--- a/os/macosx/macos.m
+++ b/os/macosx/macos.m
@@ -11,6 +11,8 @@
 #include "../../openttd.h"
 #include "../../newgrf.h"
 #include "../../gfx.h"
+#include "../../macros.h"
+#include "../../string.h"
 
 #ifndef CPU_SUBTYPE_POWERPC_970
 #define CPU_SUBTYPE_POWERPC_970 ((cpu_subtype_t) 100)
@@ -27,92 +29,86 @@
 static char *GetOSString(void)
 {
 	static char buffer[175];
-	char CPU[20];
+	const char* CPU;
 	char OS[20];
 	char newgrf[125];
-	long		sysVersion;
-    extern const char _openttd_revision[];
-
+	long sysVersion;
+	extern const char _openttd_revision[];
 
 	// get the hardware info
 	host_basic_info_data_t hostInfo;
 	mach_msg_type_number_t infoCount;
 
 	infoCount = HOST_BASIC_INFO_COUNT;
-	host_info(mach_host_self(), HOST_BASIC_INFO,
-			  (host_info_t)&hostInfo, &infoCount);
+	host_info(
+		mach_host_self(), HOST_BASIC_INFO, (host_info_t)&hostInfo, &infoCount
+	);
 
 	// replace the hardware info with strings, that tells a bit more than just an int
-#ifdef __POWERPC__
 	switch (hostInfo.cpu_subtype) {
-		case CPU_SUBTYPE_POWERPC_750:
-			sprintf(CPU,"G3");
-			break;
+#ifdef __POWERPC__
+		case CPU_SUBTYPE_POWERPC_750:  CPU = "G3"; break;
 		case CPU_SUBTYPE_POWERPC_7400:
-		case CPU_SUBTYPE_POWERPC_7450:
-			sprintf(CPU,"G4");
-			break;
-		case CPU_SUBTYPE_POWERPC_970:
-			sprintf(CPU,"G5");
-			break;
-		default:
-			sprintf(CPU,"Unknown PPC");
-	}
+		case CPU_SUBTYPE_POWERPC_7450: CPU = "G4"; break;
+		case CPU_SUBTYPE_POWERPC_970:  CPU = "G5"; break;
+		default:                       CPU = "Unknown PPC"; break;
 #else
-	// it looks odd to have a switch for two cases, but it leaves room for easy expansion. Odds are that Apple will some day use newer CPUs than i686
-	switch (hostInfo.cpu_subtype) {
-		case CPU_SUBTYPE_PENTPRO:
-			sprintf(CPU,"i686");
-			break;
-		default:
-			sprintf(CPU,"Unknown Intel");
-	}
+		/* it looks odd to have a switch for two cases, but it leaves room for easy
+		 * expansion. Odds are that Apple will some day use newer CPUs than i686
+		 */
+		case CPU_SUBTYPE_PENTPRO: CPU = "i686"; break;
+		default:                  CPU = "Unknown Intel"; break;
 #endif
+	}
 
 	// get the version of OSX
-	if( Gestalt( gestaltSystemVersion, &sysVersion ) != noErr ) {
-		sprintf(OS,"Undetected");
+	if (Gestalt(gestaltSystemVersion, &sysVersion) != noErr) {
+		sprintf(OS, "Undetected");
 	} else {
-
-		int		majorHiNib, majorLoNib, minorNib, bugNib;
-
-		majorHiNib = (sysVersion & 0x0000F000) >> 12;
-		majorLoNib = (sysVersion & 0x00000F00) >> 8;
-		minorNib = (sysVersion & 0x000000F0) >> 4;
-		bugNib = sysVersion & 0x0000000F;
+		int majorHiNib = GB(sysVersion, 12, 4);
+		int majorLoNib = GB(sysVersion,  8, 4);
+		int minorNib   = GB(sysVersion,  4, 4);
+		int bugNib     = GB(sysVersion,  0, 4);
 
 		sprintf(OS, "%d%d.%d.%d", majorHiNib, majorLoNib, minorNib, bugNib);
 	}
 
 	// make a list of used newgrf files
 	if (_first_grffile != NULL) {
-		GRFFile *file;
-		newgrf[0] = 0;
-
+		char* n = newgrf;
+		const GRFFile* file;
 
 		for (file = _first_grffile; file != NULL; file = file->next) {
-			sprintf(newgrf, "%s %s", newgrf, file->filename);
+			n = strecpy(n, " ", lastof(newgrf));
+			n = strecpy(n, file->filename, lastof(newgrf));
 		}
 	} else {
 		sprintf(newgrf, "none");
 	}
-	sprintf(buffer, "Please add this info: (tip: copy-paste works)\nCPU: %s, OSX: %s, OpenTTD version: %s\nNewGRF files:%s", CPU, OS, _openttd_revision, newgrf);
+
+	snprintf(
+		buffer, lengthof(buffer),
+		"Please add this info: (tip: copy-paste works)\n"
+		"CPU: %s, OSX: %s, OpenTTD version: %s\n"
+		"NewGRF files:%s",
+		CPU, OS, _openttd_revision, newgrf
+	);
 	return buffer;
 }
 
 
 #ifdef WITH_SDL
 
-void ShowMacDialog ( const char *title, const char *message, const char *buttonLabel )
+void ShowMacDialog(const char* title, const char* message, const char* buttonLabel)
 {
 	NSRunAlertPanel([NSString stringWithCString: title], [NSString stringWithCString: message], [NSString stringWithCString: buttonLabel], nil, nil);
 }
 
 #elif defined WITH_COCOA
 
-void CocoaDialog ( const char *title, const char *message, const char *buttonLabel );
+void CocoaDialog(const char* title, const char* message, const char* buttonLabel);
 
-void ShowMacDialog ( const char *title, const char *message, const char *buttonLabel )
+void ShowMacDialog(const char* title, const char* message, const char* buttonLabel)
 {
 	CocoaDialog(title, message, buttonLabel);
 }
@@ -120,21 +116,29 @@ void ShowMacDialog ( const char *title, const char *message, const char *buttonL
 
 #else
 
-void ShowMacDialog ( const char *title, const char *message, const char *buttonLabel )
+void ShowMacDialog(const char* title, const char* message, const char* buttonLabel)
 {
 	fprintf(stderr, "%s: %s\n", title, message);
 }
 
 #endif
 
-void ShowMacAssertDialog ( const char *function, const char *file, const int line, const char *expression )
+void ShowMacAssertDialog(const char* function, const char* file, const int line, const char* expression)
 {
-	const char *buffer =
-	[[NSString stringWithFormat:@"An assertion has failed and OpenTTD must quit.\n%s in %s (line %d)\n\"%s\"\n\nYou should report this error the OpenTTD developers if you think you found a bug.\n\n%s",
-		function, file, line, expression, GetOSString()] cString];
+	const char* buffer =
+		[[NSString stringWithFormat:@
+			"An assertion has failed and OpenTTD must quit.\n"
+			"%s in %s (line %d)\n"
+			"\"%s\"\n"
+			"\n"
+			"You should report this error the OpenTTD developers if you think you found a bug.\n"
+			"\n"
+			"%s",
+			function, file, line, expression, GetOSString()] cString
+		];
 	NSLog(@"%s", buffer);
 	ToggleFullScreen(0);
-	ShowMacDialog( "Assertion Failed", buffer, "Quit" );
+	ShowMacDialog("Assertion Failed", buffer, "Quit");
 
 	// abort so that a debugger has a chance to notice
 	abort();
@@ -143,9 +147,16 @@ void ShowMacAssertDialog ( const char *function, const char *file, const int lin
 
 void ShowMacErrorDialog(const char *error)
 {
-	const char *buffer =
-	[[NSString stringWithFormat:@"Please update to the newest version of OpenTTD\nIf the problem presists, please report this to\nhttp://bugs.openttd.org\n\n%s", GetOSString()] cString];
+	const char* buffer =
+		[[NSString stringWithFormat:@
+			"Please update to the newest version of OpenTTD\n"
+			"If the problem presists, please report this to\n"
+			"http://bugs.openttd.org\n"
+			"\n"
+			"%s",
+			GetOSString()] cString
+		];
 	ToggleFullScreen(0);
-	ShowMacDialog(error, buffer, "Quit" );
+	ShowMacDialog(error, buffer, "Quit");
 	abort();
 }