From e2e2d4b7bfea50f7c3a8adf33f3d8c36cb30f190 Mon Sep 17 00:00:00 2001 From: Ted John Date: Fri, 14 Dec 2018 17:03:48 +0000 Subject: [PATCH 1/2] Fix #8433: Crash if master server response is not valid JSON Protect fetch_servers_callback from bad JSON responses --- distribution/changelog.txt | 1 + src/openrct2-ui/windows/ServerList.cpp | 107 ++++++++++++++++--------- 2 files changed, 68 insertions(+), 40 deletions(-) diff --git a/distribution/changelog.txt b/distribution/changelog.txt index f57bbd5370..0d5e5ccd18 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -37,6 +37,7 @@ - Fix: [#8200] Incorrect behaviour when removing entrances and exits that are on the same tile. - Fix: [#8204] Crash when tile element has no surface elements. - Fix: [#8358] Infinite loop when changing vehicle count on stopped ride. +- Fix: [#8433] Crash if master server response is not valid JSON. - Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only). - Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab. - Improved: [#7930] Automatically create folders for custom content. diff --git a/src/openrct2-ui/windows/ServerList.cpp b/src/openrct2-ui/windows/ServerList.cpp index 64bbd8146c..9f75d6a5dd 100644 --- a/src/openrct2-ui/windows/ServerList.cpp +++ b/src/openrct2-ui/windows/ServerList.cpp @@ -37,6 +37,17 @@ using namespace OpenRCT2::Network; #define WHEIGHT_MAX 800 #define ITEM_HEIGHT (3 + 9 + 3) +class MasterServerException : public std::exception +{ +public: + rct_string_id StatusText; + + MasterServerException(rct_string_id statusText) + : StatusText(statusText) + { + } +}; + static char _playerName[32 + 1]; static std::vector _serverEntries; static std::mutex _mutex; @@ -136,6 +147,7 @@ static void join_server(std::string address); #ifndef DISABLE_HTTP static void fetch_servers(); static void fetch_servers_callback(Http::Response& response); +static void RefreshServersFromJson(const json_t* jsonServers); #endif static bool is_version_valid(const std::string& version); @@ -663,61 +675,75 @@ static uint32_t get_total_player_count() static void fetch_servers_callback(Http::Response& response) { - if (response.status != Http::Status::OK) + json_t* root = nullptr; + try + { + if (response.status != Http::Status::OK) + { + throw MasterServerException(STR_SERVER_LIST_NO_CONNECTION); + } + + root = Json::FromString(response.body); + auto jsonStatus = json_object_get(root, "status"); + if (!json_is_number(jsonStatus)) + { + throw MasterServerException(STR_SERVER_LIST_INVALID_RESPONSE_JSON_NUMBER); + } + + auto status = (int32_t)json_integer_value(jsonStatus); + if (status != 200) + { + throw MasterServerException(STR_SERVER_LIST_MASTER_SERVER_FAILED); + } + + auto jsonServers = json_object_get(root, "servers"); + if (!json_is_array(jsonServers)) + { + throw MasterServerException(STR_SERVER_LIST_INVALID_RESPONSE_JSON_ARRAY); + } + + RefreshServersFromJson(jsonServers); + } + catch (const MasterServerException& e) + { + status_text = e.StatusText; + window_invalidate_by_class(WC_SERVER_LIST); + } + catch (const std::exception& e) { status_text = STR_SERVER_LIST_NO_CONNECTION; window_invalidate_by_class(WC_SERVER_LIST); - log_warning("Unable to connect to master server"); - return; + log_warning("Unable to connect to master server: %s", e.what()); } - json_t* root = Json::FromString(response.body); - json_t* jsonStatus = json_object_get(root, "status"); - if (!json_is_number(jsonStatus)) + if (root != nullptr) { - status_text = STR_SERVER_LIST_INVALID_RESPONSE_JSON_NUMBER; - window_invalidate_by_class(WC_SERVER_LIST); - log_warning("Invalid response from master server"); - return; + json_decref(root); + root = nullptr; } +} - int32_t status = (int32_t)json_integer_value(jsonStatus); - if (status != 200) - { - status_text = STR_SERVER_LIST_MASTER_SERVER_FAILED; - window_invalidate_by_class(WC_SERVER_LIST); - log_warning("Master server failed to return servers"); - return; - } - - json_t* jsonServers = json_object_get(root, "servers"); - if (!json_is_array(jsonServers)) - { - status_text = STR_SERVER_LIST_INVALID_RESPONSE_JSON_ARRAY; - window_invalidate_by_class(WC_SERVER_LIST); - log_warning("Invalid response from master server"); - return; - } - - int32_t count = (int32_t)json_array_size(jsonServers); +static void RefreshServersFromJson(const json_t* jsonServers) +{ + auto count = (int32_t)json_array_size(jsonServers); for (int32_t i = 0; i < count; i++) { - json_t* server = json_array_get(jsonServers, i); + auto server = json_array_get(jsonServers, i); if (!json_is_object(server)) { continue; } - json_t* port = json_object_get(server, "port"); - json_t* name = json_object_get(server, "name"); - json_t* description = json_object_get(server, "description"); - json_t* requiresPassword = json_object_get(server, "requiresPassword"); - json_t* version = json_object_get(server, "version"); - json_t* players = json_object_get(server, "players"); - json_t* maxPlayers = json_object_get(server, "maxPlayers"); - json_t* ip = json_object_get(server, "ip"); - json_t* ip4 = json_object_get(ip, "v4"); - json_t* addressIp = json_array_get(ip4, 0); + auto port = json_object_get(server, "port"); + auto name = json_object_get(server, "name"); + auto description = json_object_get(server, "description"); + auto requiresPassword = json_object_get(server, "requiresPassword"); + auto version = json_object_get(server, "version"); + auto players = json_object_get(server, "players"); + auto maxPlayers = json_object_get(server, "maxPlayers"); + auto ip = json_object_get(server, "ip"); + auto ip4 = json_object_get(ip, "v4"); + auto addressIp = json_array_get(ip4, 0); if (name == nullptr || version == nullptr) { @@ -744,6 +770,7 @@ static void fetch_servers_callback(Http::Response& response) status_text = STR_X_PLAYERS_ONLINE; window_invalidate_by_class(WC_SERVER_LIST); } + #endif static bool is_version_valid(const std::string& version) From fecf4ac3e88d50e4ddbc0028dba6b00cea07b3e3 Mon Sep 17 00:00:00 2001 From: Ted John Date: Fri, 14 Dec 2018 17:10:34 +0000 Subject: [PATCH 2/2] Fix #8434: Crash if curl_easy_init fails --- distribution/changelog.txt | 1 + src/openrct2/network/Http.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/distribution/changelog.txt b/distribution/changelog.txt index 0d5e5ccd18..579e32b8f0 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -38,6 +38,7 @@ - Fix: [#8204] Crash when tile element has no surface elements. - Fix: [#8358] Infinite loop when changing vehicle count on stopped ride. - Fix: [#8433] Crash if master server response is not valid JSON. +- Fix: [#8434] Crash if curl_easy_init fails. - Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only). - Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab. - Improved: [#7930] Automatically create folders for custom content. diff --git a/src/openrct2/network/Http.cpp b/src/openrct2/network/Http.cpp index ee4f30e4f1..8dc5eae94b 100644 --- a/src/openrct2/network/Http.cpp +++ b/src/openrct2/network/Http.cpp @@ -98,7 +98,7 @@ namespace OpenRCT2::Network::Http std::shared_ptr _(nullptr, [curl](...) { curl_easy_cleanup(curl); }); if (!curl) - std::runtime_error("Failed to initialize curl"); + throw std::runtime_error("Failed to initialize curl"); Response res; WriteThis wt;