widelands-dev team mailing list archive

[Arty <diego.scheide@xxxxxx>]

Arty has proposed merging lp:~widelands-dev/widelands/bugfix-1805042-S2map-preloader-checks into lp:widelands.

Commit message:
validity checks for S2 map headers to avoid crashes when preloading invalid S2 map files

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1805042 in widelands: "segfault when trying to preload invalid file"
  https://bugs.launchpad.net/widelands/+bug/1805042

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bugfix-1805042-S2map-preloader-checks/+merge/360762
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bugfix-1805042-S2map-preloader-checks into lp:widelands.

=== modified file 'src/map_io/s2map.cc'
--- src/map_io/s2map.cc	2018-04-11 06:55:01 +0000
+++ src/map_io/s2map.cc	2018-12-12 00:02:20 +0000
@@ -61,6 +61,29 @@
 	char bulk[2290];  // unknown
 } /* size 2352 */;
 
+// Some basic checks to identify obviously invalid headers
+bool is_valid_header(const S2MapDescrHeader& header) {
+	if (strncmp(header.magic, "WORLD_V1.0", 10)) {
+		return false;
+	}
+	if (header.name[19]) {
+		return false;
+	}
+	if (header.w <= 0 || header.h <= 0) {
+		return false;
+	}
+	if (header.uses_world < 0 || header.uses_world > 2) {
+		return false;
+	}
+	if (header.nplayers < 0 || header.nplayers > 7) {
+		return false;
+	}
+	if (header.author[19]) {
+		return false;
+	}
+	return true;
+}
+
 // TODO(unknown): the following bob types appear in S2 maps but are unknown
 //  Somebody who can run Settlers II please check them out
 //  11 (0x0B)
@@ -400,9 +423,11 @@
 }
 
 /**
- * Load informational data of an S2 map
+ * Loads informational data of an S2 map.
+ * Throws exception if data is invalid.
  */
 void S2MapLoader::load_s2mf_header(FileRead& fr) {
+	// no need to check file size: fr.data(..) already throws if the file is too small
 	S2MapDescrHeader header;
 	memcpy(&header, fr.data(sizeof(header)), sizeof(header));
 
@@ -414,6 +439,11 @@
 	header.h = swap_16(header.h);
 #endif
 
+	// Check header validity to prevent unexpected crashes later
+	if (!is_valid_header(header)) {
+		throw wexception("invalid S2 file");
+	}
+
 	//  don't really set size, but make the structures valid
 	map_.width_ = header.w;
 	map_.height_ = header.h;