From ce1ca1b62507ebcf50731d930f67d4c3cfa4af46 Mon Sep 17 00:00:00 2001 From: "Daniel J. Hofmann" Date: Wed, 2 Mar 2016 14:48:35 +0100 Subject: [PATCH] Fixes critical error in table plugin accessing uninitialized memory Although we check for valid coordinates in the table plugin via `check_all_coordinates`, we do not check for #srcs > 0 and #dsts > 0. This would be fine as the grammar parser combines adding coordinates and setting their `is_source` and `is_destination` property, which makes adding coordinates without specifying source or destination impossible. See: route_parameters.cpp, AddSource, AddDestination, and api_grammar.hpp In contract, the Polyline codepath does not do this! In fact, it only lets you set coordinates, but not their `is_source` or `is_destination` property. See: route_parameters.cpp, SetCoordinatesFromGeometry Therefore, the following queries only set coordinates: http 'http://localhost:5000/table?locs=s_hhFg{arEgEfEgEfEgEfEgEfEgEfEgEfEgEfEgEfEgEfE' http 'http://localhost:5000/table?locs=_p~iF~ps|U_ulLnnqC_mqNvxq`@' but fail to specify sources and targets! The distance table plugin now assumes `is_course` and `is_destination` is the same size as `coordinates`. And happily accesses uninitialized memory. --- include/engine/plugins/distance_table.hpp | 38 ++++++++++++++++------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/include/engine/plugins/distance_table.hpp b/include/engine/plugins/distance_table.hpp index 1c6d5093f..75c03db2f 100644 --- a/include/engine/plugins/distance_table.hpp +++ b/include/engine/plugins/distance_table.hpp @@ -59,18 +59,31 @@ template class DistanceTablePlugin final : public BasePlugin return Status::Error; } - auto number_of_sources = - std::count_if(route_parameters.is_source.begin(), route_parameters.is_source.end(), - [](const bool is_source) - { - return is_source; - }); - auto number_of_destination = - std::count_if(route_parameters.is_destination.begin(), - route_parameters.is_destination.end(), [](const bool is_destination) - { - return is_destination; - }); + // The check_all_coordinates guard above makes sure we have at least 2 coordinates. + // This guard makes sure is_source, is_destination, coordinates are parallel arrays. + if (route_parameters.is_source.size() != route_parameters.coordinates.size() || + route_parameters.is_destination.size() != route_parameters.coordinates.size()) + { + json_result.values["status_message"] = + "Number of sources and destinations does not match number of coordinates"; + return Status::Error; + } + + const auto number_of_sources = std::count(route_parameters.is_source.begin(), // + route_parameters.is_source.end(), true); + const auto number_of_destination = std::count(route_parameters.is_destination.begin(), // + route_parameters.is_destination.end(), true); + + // At this point we know that we + // - have at least n=2 coordinates and + // - have n booleans in is_source and n booleans in is_destination + // This guard makes sure we have at least one source and one target. + if (number_of_sources < 1 || number_of_destination < 1) + { + json_result.values["status_message"] = + "At least one source and one destination required"; + return Status::Error; + } if (max_locations_distance_table > 0 && (number_of_sources * number_of_destination > @@ -123,6 +136,7 @@ template class DistanceTablePlugin final : public BasePlugin const int range = input_bearings.size() > 0 ? (input_bearings[i].second ? *input_bearings[i].second : 10) : 180; + if (route_parameters.is_source[i]) { *phantom_node_source_out_iter =