diff --git a/CHANGELOG.md b/CHANGELOG.md index 5882d83..7aa4594 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## 1.0.2 + +- Add support for QNX 7 compiler. + +## 1.0.1 + +- Limit possibility rare situation where it might be possible to have an overflow or underflow of integers while decoding a vector tile's geometry + +## 1.0.0 + +- Allow nulls from property parsing +- Several bug fixes around geometry parsing to prevent over allocation of memory. + ## 1.0.0-alpha.1 - Added demo application showing usage of decoder diff --git a/demo/Makefile b/demo/Makefile index c66450e..607d318 100644 --- a/demo/Makefile +++ b/demo/Makefile @@ -1,6 +1,6 @@ CC := $(CC) CXX := $(CXX) -CXXFLAGS := $(CXXFLAGS) -Iinclude -std=c++14 -Wall +CXXFLAGS := $(CXXFLAGS) -isystem../include -isystem ../mason_packages/.link/include/ -std=c++14 -Wall RELEASE_FLAGS := -O3 -DNDEBUG -fvisibility-inlines-hidden -fvisibility=hidden DEBUG_FLAGS := -g -O0 -DDEBUG -fno-inline-functions -fno-omit-frame-pointer diff --git a/include/mapbox/vector_tile.hpp b/include/mapbox/vector_tile.hpp index 1e9a3de..5ddae48 100644 --- a/include/mapbox/vector_tile.hpp +++ b/include/mapbox/vector_tile.hpp @@ -12,6 +12,7 @@ #include #include + template using optional = std::experimental::optional; @@ -22,13 +23,15 @@ using point_type = mapbox::geometry::point; class points_array_type : public std::vector { public: using coordinate_type = point_type::coordinate_type; - using std::vector::vector; + template + points_array_type(Args&&... args) : std::vector(std::forward(args)...) {} }; class points_arrays_type : public std::vector { public: using coordinate_type = points_array_type::coordinate_type; - using std::vector::vector; + template + points_arrays_type(Args&&... args) : std::vector(std::forward(args)...) {} }; class layer; @@ -91,30 +94,38 @@ class buffer { }; static mapbox::geometry::value parseValue(protozero::data_view const& value_view) { + mapbox::geometry::value value; protozero::pbf_reader value_reader(value_view); while (value_reader.next()) { switch (value_reader.tag()) { case ValueType::STRING: - return value_reader.get_string(); + value = value_reader.get_string(); + break; case ValueType::FLOAT: - return static_cast(value_reader.get_float()); + value = static_cast(value_reader.get_float()); + break; case ValueType::DOUBLE: - return value_reader.get_double(); + value = value_reader.get_double(); + break; case ValueType::INT: - return value_reader.get_int64(); + value = value_reader.get_int64(); + break; case ValueType::UINT: - return value_reader.get_uint64(); + value = value_reader.get_uint64(); + break; case ValueType::SINT: - return value_reader.get_sint64(); + value = value_reader.get_sint64(); + break; case ValueType::BOOL: - return value_reader.get_bool(); + value = value_reader.get_bool(); + break; default: value_reader.skip(); break; } } - return false; + return value; } inline feature::feature(protozero::data_view const& feature_view, layer const& l) @@ -128,7 +139,7 @@ inline feature::feature(protozero::data_view const& feature_view, layer const& l while (feature_pbf.next()) { switch (feature_pbf.tag()) { case FeatureType::ID: - id = { feature_pbf.get_uint64() }; + id = optional{ feature_pbf.get_uint64() }; break; case FeatureType::TAGS: tags_iter = feature_pbf.get_packed_uint32(); @@ -215,8 +226,8 @@ template GeometryCollectionType feature::getGeometries(float scale) const { std::uint8_t cmd = 1; std::uint32_t length = 0; - std::int32_t x = 0; - std::int32_t y = 0; + std::int64_t x = 0; + std::int64_t y = 0; GeometryCollectionType paths; @@ -239,6 +250,15 @@ GeometryCollectionType feature::getGeometries(float scale) const { std::uint32_t cmd_length = static_cast(*start_itr++); cmd = cmd_length & 0x7; length = len_reserve = cmd_length >> 3; + // Prevents the creation of vector tiles that would cause + // a denial of service from massive over allocation. Protection + // limit is based on the assumption of an int64_t point which is + // 16 bytes in size and wanting to have a maximum of 1 MB of memory + // used. + constexpr std::uint32_t MAX_LENGTH = (1024 * 1024) / 16; + if (len_reserve > MAX_LENGTH) { + len_reserve = MAX_LENGTH; + } } --length; @@ -260,6 +280,13 @@ GeometryCollectionType feature::getGeometries(float scale) const { } if (cmd == CommandType::MOVE_TO && !paths.back().empty()) { + if (paths.back().size() < paths.back().capacity()) { + // Assuming we had an invalid length before + // lets shrink to fit, just to make sure + // we don't have a large capacity vector + // just wasting memory + paths.back().shrink_to_fit(); + } paths.emplace_back(); if (!is_point) { first = true; @@ -288,10 +315,18 @@ GeometryCollectionType feature::getGeometries(float scale) const { if (!paths.back().empty()) { paths.back().push_back(paths.back()[0]); } + length = 0; } else { throw std::runtime_error("unknown command"); } } + if (paths.size() < paths.capacity()) { + // Assuming we had an invalid length before + // lets shrink to fit, just to make sure + // we don't have a large capacity vector + // just wasting memory + paths.shrink_to_fit(); + } #if defined(DEBUG) for (auto const& p : paths) { assert(p.size() == p.capacity()); diff --git a/include/mapbox/vector_tile/version.hpp b/include/mapbox/vector_tile/version.hpp index e85b1f4..cc1974d 100644 --- a/include/mapbox/vector_tile/version.hpp +++ b/include/mapbox/vector_tile/version.hpp @@ -7,10 +7,10 @@ #define VECTOR_TILE_VERSION_MINOR 0 /// The patch number -#define VECTOR_TILE_VERSION_PATCH 0 +#define VECTOR_TILE_VERSION_PATCH 2 /// The complete version number #define VECTOR_TILE_VERSION_CODE (VECTOR_TILE_VERSION_MAJOR * 10000 + VECTOR_TILE_VERSION_MINOR * 100 + VECTOR_TILE_VERSION_PATCH) /// Version number as string -#define VECTOR_TILE_VERSION_STRING "1.0.0-rc7" +#define VECTOR_TILE_VERSION_STRING "1.0.2" diff --git a/package.json b/package.json index 3eca121..2370a12 100644 --- a/package.json +++ b/package.json @@ -1,10 +1,10 @@ { "name": "@mapbox/vector-tile", - "version": "1.0.0-alpha.1", + "version": "1.0.0", "description": "A C++ header only library for decoding and encoding Mapbox Vector Tiles", "main": "./include_dirs.js", "repository" : { "type" : "git", "url" : "git://github.com/mapbox/vector-tile.git" } -} \ No newline at end of file +} diff --git a/test/test046.mvt b/test/test046.mvt new file mode 100644 index 0000000..55a11b1 --- /dev/null +++ b/test/test046.mvt @@ -0,0 +1,3 @@ +;x0 + +000000000000000"0000000(€000000"‰ûûíÿ000000000 \ No newline at end of file diff --git a/test/unit/tags.test.cpp b/test/unit/tags.test.cpp index 2427afd..1c0dcdd 100644 --- a/test/unit/tags.test.cpp +++ b/test/unit/tags.test.cpp @@ -4,8 +4,8 @@ #include TEST_CASE( "Version constant" ) { - CHECK(std::string(VECTOR_TILE_VERSION_STRING) == std::string("1.0.0-rc7")); - CHECK(VECTOR_TILE_VERSION_CODE == 10000); + CHECK(std::string(VECTOR_TILE_VERSION_STRING) == std::string("1.0.2")); + CHECK(VECTOR_TILE_VERSION_CODE == 10002); } TEST_CASE( "Protobuf Tag Constants" ) { diff --git a/test/unit/vector_tile.test.cpp b/test/unit/vector_tile.test.cpp index 3e940ff..87d37d2 100644 --- a/test/unit/vector_tile.test.cpp +++ b/test/unit/vector_tile.test.cpp @@ -101,4 +101,16 @@ TEST_CASE( "Read Feature-single-polygon.mvt" ) { REQUIRE(stringify_geom(geom) == "25, 17"); }*/ - +TEST_CASE( "Prevent massive over allocation" ) { + std::string buffer = open_tile("test/test046.mvt"); + mapbox::vector_tile::buffer tile(buffer); + auto const layer_names = tile.layerNames(); + REQUIRE(layer_names.size() == 1); + REQUIRE(layer_names[0] == "0000000000"); + auto const layer = tile.getLayer("0000000000"); + REQUIRE(layer.featureCount() == 1); + REQUIRE(layer.getName() == "0000000000"); + auto const feature = mapbox::vector_tile::feature(layer.getFeature(0),layer); + mapbox::vector_tile::points_arrays_type geom = feature.getGeometries(1.0); + REQUIRE(geom.capacity() <= 655360); +}