Merge pull request #481 from mapbox/crashers

Be more careful about corrupt mbtiles files

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.26.3
+
+* Guard against impossible coordinates when decoding tilesets
+
 ## 1.26.2
 
 * Make sure to encode tile-joined integers as ints, not doubles

decode.cpp

@@ -93,6 +93,11 @@ void handle(std::string message, int z, unsigned x, unsigned y, int describe, st
 	for (size_t l = 0; l < tile.layers.size(); l++) {
 		mvt_layer &layer = tile.layers[l];
 
+		if (layer.extent <= 0) {
+			fprintf(stderr, "Impossible layer extent %lld in mbtiles\n", layer.extent);
+			exit(EXIT_FAILURE);
+		}
+
 		if (to_decode.size() != 0 && !to_decode.count(layer.name)) {
 			continue;
 		}
@@ -114,6 +119,12 @@ void handle(std::string message, int z, unsigned x, unsigned y, int describe, st
 			}
 		}
 
+		// X and Y are unsigned, so no need to check <0
+		if (x > (1 << z) || y > (1 << z)) {
+			fprintf(stderr, "Impossible tile %d/%u/%u\n", z, x, y);
+			exit(EXIT_FAILURE);
+		}
+
 		layer_to_geojson(stdout, layer, z, x, y, !pipeline, pipeline, pipeline, 0, 0, 0, !force);
 
 		if (!pipeline) {
@@ -192,6 +203,11 @@ void decode(char *fname, int z, unsigned x, unsigned y, std::set<std::string> co
 				const unsigned char *name = sqlite3_column_text(stmt2, 0);
 				const unsigned char *value = sqlite3_column_text(stmt2, 1);
 
+				if (name == NULL || value == NULL) {
+					fprintf(stderr, "Corrupt mbtiles file: null metadata\n");
+					exit(EXIT_FAILURE);
+				}
+
 				fprintq(stdout, (char *) name);
 				printf(": ");
 				fprintq(stdout, (char *) value);
@@ -237,6 +253,12 @@ void decode(char *fname, int z, unsigned x, unsigned y, std::set<std::string> co
 			int tz = sqlite3_column_int(stmt, 1);
 			int tx = sqlite3_column_int(stmt, 2);
 			int ty = sqlite3_column_int(stmt, 3);
+
+			if (tz < 0 || tz >= 32) {
+				fprintf(stderr, "Impossible zoom level %d in mbtiles\n", tz);
+				exit(EXIT_FAILURE);
+			}
+
 			ty = (1LL << tz) - 1 - ty;
 			const char *s = (const char *) sqlite3_column_blob(stmt, 0);
 

enumerate.cpp

@@ -24,6 +24,11 @@ void enumerate(char *fname) {
 		long long x = sqlite3_column_int(stmt, 1);
 		long long y = sqlite3_column_int(stmt, 2);
 
+		if (zoom < 0 || zoom > 31) {
+			fprintf(stderr, "Corrupt mbtiles file: impossible zoom level %lld\n", zoom);
+			exit(EXIT_FAILURE);
+		}
+
 		y = (1LL << zoom) - 1 - y;
 		printf("%s %lld %lld %lld\n", fname, zoom, x, y);
 	}

mbtiles.cpp

@@ -501,8 +501,15 @@ void mbtiles_write_metadata(sqlite3 *outdb, const char *outdir, const char *fnam
 			while (sqlite3_step(stmt) == SQLITE_ROW) {
 				std::string key, value;
 
-				quote(key, (const char *) sqlite3_column_text(stmt, 0));
+				const char *k = (const char *) sqlite3_column_text(stmt, 0);
-				quote(value, (const char *) sqlite3_column_text(stmt, 1));
+				const char *v = (const char *) sqlite3_column_text(stmt, 1);
+				if (k == NULL || v == NULL) {
+					fprintf(stderr, "Corrupt mbtiles file: null metadata\n");
+					exit(EXIT_FAILURE);
+				}
+
+				quote(key, k);
+				quote(value, v);
 
 				if (!first) {
 					fprintf(fp, ",\n");

tile-join.cpp

@@ -837,28 +837,39 @@ void decode(struct reader *readers, char *map, std::map<std::string, layermap_en
 			if (sqlite3_prepare_v2(r->db, "SELECT value from metadata where name = 'center'", -1, &r->stmt, NULL) == SQLITE_OK) {
 				if (sqlite3_step(r->stmt) == SQLITE_ROW) {
 					const unsigned char *s = sqlite3_column_text(r->stmt, 0);
-					sscanf((char *) s, "%lf,%lf", &st->midlon, &st->midlat);
+					if (s != NULL) {
+						sscanf((char *) s, "%lf,%lf", &st->midlon, &st->midlat);
+					}
 				}
 				sqlite3_finalize(r->stmt);
 			}
 			if (sqlite3_prepare_v2(r->db, "SELECT value from metadata where name = 'attribution'", -1, &r->stmt, NULL) == SQLITE_OK) {
 				if (sqlite3_step(r->stmt) == SQLITE_ROW) {
-					attribution = std::string((char *) sqlite3_column_text(r->stmt, 0));
+					const unsigned char *s = sqlite3_column_text(r->stmt, 0);
+					if (s != NULL) {
+						attribution = std::string((char *) s);
+					}
 				}
 				sqlite3_finalize(r->stmt);
 			}
 			if (sqlite3_prepare_v2(r->db, "SELECT value from metadata where name = 'description'", -1, &r->stmt, NULL) == SQLITE_OK) {
 				if (sqlite3_step(r->stmt) == SQLITE_ROW) {
-					description = std::string((char *) sqlite3_column_text(r->stmt, 0));
+					const unsigned char *s = sqlite3_column_text(r->stmt, 0);
+					if (s != NULL) {
+						description = std::string((char *) s);
+					}
 				}
 				sqlite3_finalize(r->stmt);
 			}
 			if (sqlite3_prepare_v2(r->db, "SELECT value from metadata where name = 'name'", -1, &r->stmt, NULL) == SQLITE_OK) {
 				if (sqlite3_step(r->stmt) == SQLITE_ROW) {
-					if (name.size() == 0) {
+					const unsigned char *s = sqlite3_column_text(r->stmt, 0);
-						name = std::string((char *) sqlite3_column_text(r->stmt, 0));
+					if (s != NULL) {
-					} else {
+						if (name.size() == 0) {
-						name += " + " + std::string((char *) sqlite3_column_text(r->stmt, 0));
+							name = std::string((char *) s);
+						} else {
+							name += " + " + std::string((char *) s);
+						}
 					}
 				}
 				sqlite3_finalize(r->stmt);
@@ -866,11 +877,13 @@ void decode(struct reader *readers, char *map, std::map<std::string, layermap_en
 			if (sqlite3_prepare_v2(r->db, "SELECT value from metadata where name = 'bounds'", -1, &r->stmt, NULL) == SQLITE_OK) {
 				if (sqlite3_step(r->stmt) == SQLITE_ROW) {
 					const unsigned char *s = sqlite3_column_text(r->stmt, 0);
-					if (sscanf((char *) s, "%lf,%lf,%lf,%lf", &minlon, &minlat, &maxlon, &maxlat) == 4) {
+					if (s != NULL) {
-						st->minlon = min(minlon, st->minlon);
+						if (sscanf((char *) s, "%lf,%lf,%lf,%lf", &minlon, &minlat, &maxlon, &maxlat) == 4) {
-						st->maxlon = max(maxlon, st->maxlon);
+							st->minlon = min(minlon, st->minlon);
-						st->minlat = min(minlat, st->minlat);
+							st->maxlon = max(maxlon, st->maxlon);
-						st->maxlat = max(maxlat, st->maxlat);
+							st->minlat = min(minlat, st->minlat);
+							st->maxlat = max(maxlat, st->maxlat);
+						}
 					}
 				}
 				sqlite3_finalize(r->stmt);

version.hpp

@@ -1,6 +1,6 @@
 #ifndef VERSION_HPP
 #define VERSION_HPP
 
-#define VERSION "tippecanoe v1.26.2\n"
+#define VERSION "tippecanoe v1.26.3\n"
 
 #endif