diff options
| -rw-r--r-- | src/httpserver.cpp | 3 | ||||
| -rw-r--r-- | src/rest.cpp | 26 | ||||
| -rw-r--r-- | src/test/httpserver_tests.cpp | 4 | ||||
| -rwxr-xr-x | test/functional/interface_rest.py | 4 |
4 files changed, 33 insertions, 4 deletions
diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 942caa042d..8e49f9c0f4 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -673,6 +673,9 @@ std::optional<std::string> HTTPRequest::GetQueryParameter(const std::string& key std::optional<std::string> GetQueryParameterFromUri(const char* uri, const std::string& key) { evhttp_uri* uri_parsed{evhttp_uri_parse(uri)}; + if (!uri_parsed) { + throw std::runtime_error("URI parsing failed, it likely contained RFC 3986 invalid characters"); + } const char* query{evhttp_uri_get_query(uri_parsed)}; std::optional<std::string> result; diff --git a/src/rest.cpp b/src/rest.cpp index fa119ddfb7..e46406f1ad 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -200,7 +200,11 @@ static bool rest_headers(const std::any& context, } else if (path.size() == 1) { // new path with query parameter: /rest/headers/<hash>?count=<count> hashStr = path[0]; - raw_count = req->GetQueryParameter("count").value_or("5"); + try { + raw_count = req->GetQueryParameter("count").value_or("5"); + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } } else { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/headers/<hash>.<ext>?count=<count>"); } @@ -371,7 +375,11 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const } else if (uri_parts.size() == 2) { // new path with query parameter: /rest/blockfilterheaders/<filtertype>/<blockhash>?count=<count> raw_blockhash = uri_parts[1]; - raw_count = req->GetQueryParameter("count").value_or("5"); + try { + raw_count = req->GetQueryParameter("count").value_or("5"); + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } } else { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders/<filtertype>/<blockhash>.<ext>?count=<count>"); } @@ -652,11 +660,21 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s case RESTResponseFormat::JSON: { std::string str_json; if (param == "contents") { - const std::string raw_verbose{req->GetQueryParameter("verbose").value_or("true")}; + std::string raw_verbose; + try { + raw_verbose = req->GetQueryParameter("verbose").value_or("true"); + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } if (raw_verbose != "true" && raw_verbose != "false") { return RESTERR(req, HTTP_BAD_REQUEST, "The \"verbose\" query parameter must be either \"true\" or \"false\"."); } - const std::string raw_mempool_sequence{req->GetQueryParameter("mempool_sequence").value_or("false")}; + std::string raw_mempool_sequence; + try { + raw_mempool_sequence = req->GetQueryParameter("mempool_sequence").value_or("false"); + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } if (raw_mempool_sequence != "true" && raw_mempool_sequence != "false") { return RESTERR(req, HTTP_BAD_REQUEST, "The \"mempool_sequence\" query parameter must be either \"true\" or \"false\"."); } diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index ee59ec6967..c95a777e80 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -34,5 +34,9 @@ BOOST_AUTO_TEST_CASE(test_query_parameters) // Invalid query string syntax is the same as not having parameters uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2"; BOOST_CHECK(!GetQueryParameterFromUri(uri.c_str(), "p1").has_value()); + + // URI with invalid characters (%) raises a runtime error regardless of which query parameter is queried + uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2%"; + BOOST_CHECK_EXCEPTION(GetQueryParameterFromUri(uri.c_str(), "p1"), std::runtime_error, HasReason("URI parsing failed, it likely contained RFC 3986 invalid characters")); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 23109093d6..869bd5c235 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -277,6 +277,10 @@ class RESTTest (BitcoinTestFramework): assert_equal(len(json_obj), 1) # ensure that there is one header in the json response assert_equal(json_obj[0]['hash'], bb_hash) # request/response hash should be the same + # Check invalid uri (% symbol at the end of the request) + resp = self.test_rest_request(f"/headers/{bb_hash}%", ret_type=RetType.OBJ, status=400) + assert_equal(resp.read().decode('utf-8').rstrip(), "URI parsing failed, it likely contained RFC 3986 invalid characters") + # Compare with normal RPC block response rpc_block_json = self.nodes[0].getblock(bb_hash) for key in ['hash', 'confirmations', 'height', 'version', 'merkleroot', 'time', 'nonce', 'bits', 'difficulty', 'chainwork', 'previousblockhash']: |