From 30e3a2d79433b9722569a3a8ed05eb73e00dcf82 Mon Sep 17 00:00:00 2001 From: nasr <156965421+div0rce@users.noreply.github.com> Date: Wed, 24 Jun 2026 20:52:44 -0400 Subject: [PATCH] fix(apps): reject malformed CLI numeric args instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug-hunt found three CLI tools parsed numeric arguments with unguarded std::stoul/std::stoull/std::stoi, which call std::terminate() (an uncaught std::invalid_argument/std::out_of_range escapes main) on non-numeric or out-of-range input, and silently truncate out-of-range ports via a uint16_t cast: - qsl-client - qsl-mdfeed subscribe [count] [rcvbuf] / publish [seed] [orders] - qsl-export-fixture [seed] [orders] All now parse with std::from_chars helpers (parse_u64/parse_port/parse_rcvbuf, matching qsl-replay's exception-free style), require the whole token to be consumed, and bound ports to 0-65535; on any malformed arg they print usage and exit 2 instead of aborting. (qsl-gateway/qsl-replay/qsl-export-stream already did this; this closes the gap in the remaining tools.) Refactored each main into small parse helpers (port_from_args / run_subscribe / run_publish / parse_args, plus an all_present fold) so the added validation stays under the CodeScene complexity/conditional thresholds — net Code Health improved (mdfeed 9.50->9.68, export-fixture 9.51->9.53), delta clean. Tests: 5 CTest cases assert malformed/out-of-range args exit non-zero with usage (qsl_client_invalid_port_fails, qsl_client_out_of_range_port_fails, qsl_mdfeed_invalid_count_fails, qsl_mdfeed_out_of_range_port_fails, qsl_export_fixture_invalid_seed_fails). make check/asan 270/270. Co-Authored-By: Claude Opus 4.8 --- apps/qsl-client/main.cpp | 32 ++++++++++++- apps/qsl-export-fixture/main.cpp | 40 ++++++++++++++++- apps/qsl-mdfeed/main.cpp | 77 +++++++++++++++++++++++++++----- tests/CMakeLists.txt | 32 +++++++++++++ 4 files changed, 167 insertions(+), 14 deletions(-) diff --git a/apps/qsl-client/main.cpp b/apps/qsl-client/main.cpp index 04c4efe..7cddd4f 100644 --- a/apps/qsl-client/main.cpp +++ b/apps/qsl-client/main.cpp @@ -2,18 +2,43 @@ #include #include +#include #include #include #include +#include #include +#include #include #include +#include #include #include #include namespace { +// Parse a whole token as a TCP port (0-65535). Rejects junk, trailing characters, and out-of-range +// values instead of std::stoul's throw-or-silently-truncate behavior. +std::optional parse_port(std::string_view s) { + std::uint32_t value = 0; + const char *begin = s.data(); + const char *end = begin + s.size(); + const auto [ptr, ec] = std::from_chars(begin, end, value); + if (ec != std::errc{} || ptr != end || value > std::numeric_limits::max()) { + return std::nullopt; + } + return static_cast(value); +} + +// The optional port arg defaults to 9009; nullopt means it was present but malformed. +std::optional port_from_args(int argc, char **argv) { + if (argc < 2) { + return std::uint16_t{9009}; + } + return parse_port(argv[1]); +} + bool write_all(int fd, const std::vector &data) { std::size_t offset = 0; while (offset < data.size()) { @@ -73,7 +98,12 @@ void print_responses(std::span bytes) { // qsl-client [port] -> connect to 127.0.0.1:port, send a NewOrder and a Heartbeat, print replies. int main(int argc, char **argv) { - const std::uint16_t port = (argc >= 2) ? static_cast(std::stoul(argv[1])) : 9009; + const auto port_opt = port_from_args(argc, argv); + if (!port_opt) { + std::cerr << "usage: qsl-client [port] (one optional port, 0-65535)\n"; + return 2; + } + const std::uint16_t port = *port_opt; const int fd = ::socket(AF_INET, SOCK_STREAM, 0); if (fd < 0) { diff --git a/apps/qsl-export-fixture/main.cpp b/apps/qsl-export-fixture/main.cpp index 47ca8fd..cb3cb13 100644 --- a/apps/qsl-export-fixture/main.cpp +++ b/apps/qsl-export-fixture/main.cpp @@ -4,10 +4,13 @@ #include "qsl/replay/dispatch.hpp" #include "qsl/replay/recovery.hpp" +#include #include #include #include +#include #include +#include #include #include @@ -15,6 +18,34 @@ using namespace qsl; namespace { +// Parse a whole token as an unsigned integer; rejects junk/trailing chars/overflow rather than +// std::stoull's throw-on-bad-input (which would call std::terminate from main). +std::optional parse_u64(std::string_view s) { + std::uint64_t value = 0; + const char *begin = s.data(); + const char *end = begin + s.size(); + const auto [ptr, ec] = std::from_chars(begin, end, value); + if (ec != std::errc{} || ptr != end) { + return std::nullopt; + } + return value; +} + +struct FixtureArgs { + std::uint64_t seed; + std::size_t orders; +}; + +// Optional [seed] [orders] args (defaults 42 / 200); nullopt if either is present but malformed. +std::optional parse_args(int argc, char **argv) { + const auto seed = (argc >= 2) ? parse_u64(argv[1]) : std::optional{42}; + const auto orders = (argc >= 3) ? parse_u64(argv[2]) : std::optional{200}; + if (!seed || !orders) { + return std::nullopt; + } + return FixtureArgs{*seed, static_cast(*orders)}; +} + // Emit one normalized line per engine event (in emission order). void emit_events(std::ostream &os, const std::vector &events) { for (const auto &event : events) { @@ -45,8 +76,13 @@ void emit_events(std::ostream &os, const std::vector &event // max_order_quantity makes some new orders reject, so the fixture exercises the // "rejected order never rests" invariant. Engine-neutral: no engine code is modified. int main(int argc, char **argv) { - const std::uint64_t seed = (argc >= 2) ? std::stoull(argv[1]) : 42; - const std::size_t orders = (argc >= 3) ? std::stoull(argv[2]) : 200; + const auto args = parse_args(argc, argv); + if (!args) { + std::cerr << "usage: qsl-export-fixture [seed] [orders]\n"; + return 2; + } + const std::uint64_t seed = args->seed; + const std::size_t orders = args->orders; const core::SymbolId symbols = 4; const core::Quantity max_qty = 8; // generate_flow can emit qty > 8 -> some orders reject diff --git a/apps/qsl-mdfeed/main.cpp b/apps/qsl-mdfeed/main.cpp index 79c17a2..a0a21a0 100644 --- a/apps/qsl-mdfeed/main.cpp +++ b/apps/qsl-mdfeed/main.cpp @@ -4,14 +4,57 @@ #include "qsl/feed/udp_feed.hpp" #include "qsl/replay/recovery.hpp" +#include #include #include #include +#include +#include #include +#include #include namespace { +// Parse a whole token as an unsigned integer, rejecting junk/trailing chars/overflow (unlike +// std::stoul/std::stoull which throw on bad input and std::stoi which can wrap a port silently). +std::optional parse_u64(std::string_view s) { + std::uint64_t value = 0; + const char *begin = s.data(); + const char *end = begin + s.size(); + const auto [ptr, ec] = std::from_chars(begin, end, value); + if (ec != std::errc{} || ptr != end) { + return std::nullopt; + } + return value; +} + +std::optional parse_port(std::string_view s) { + const auto value = parse_u64(s); + if (!value || *value > std::numeric_limits::max()) { + return std::nullopt; + } + return static_cast(*value); +} + +std::optional parse_rcvbuf(std::string_view s) { + const auto value = parse_u64(s); + if (!value || *value > std::numeric_limits::max()) { + return std::nullopt; + } + return static_cast(*value); +} + +template bool all_present(const Opts &...opts) noexcept { + return (static_cast(opts) && ...); +} + +int usage() { + std::cerr << "usage:\n qsl-mdfeed subscribe [count] [rcvbuf_bytes]\n qsl-mdfeed " + "publish [seed] [orders]\n"; + return 2; +} + int publish(std::uint16_t port, std::uint64_t seed, std::size_t orders) { qsl::engine::MatchingEngine engine; qsl::feed::MarketDataPublisher publisher; @@ -74,22 +117,34 @@ int subscribe(std::uint16_t port, std::size_t count, int recv_buffer_bytes) { } // namespace +int run_subscribe(int argc, char **argv) { + const auto port = parse_port(argv[2]); + const auto count = (argc >= 4) ? parse_u64(argv[3]) : std::optional{20}; + const auto rcvbuf = (argc >= 5) ? parse_rcvbuf(argv[4]) : std::optional{0}; + if (!all_present(port, count, rcvbuf)) { + return usage(); + } + return subscribe(*port, static_cast(*count), *rcvbuf); +} + +int run_publish(int argc, char **argv) { + const auto port = parse_port(argv[2]); + const auto seed = (argc >= 4) ? parse_u64(argv[3]) : std::optional{42}; + const auto orders = (argc >= 5) ? parse_u64(argv[4]) : std::optional{200}; + if (!all_present(port, seed, orders)) { + return usage(); + } + return publish(*port, *seed, static_cast(*orders)); +} + // qsl-mdfeed subscribe [count] [rcvbuf_bytes] -> receive datagrams and report gaps // qsl-mdfeed publish [seed] [orders] -> run a synthetic flow, stream its data int main(int argc, char **argv) { if (argc >= 3 && std::string(argv[1]) == "subscribe") { - const auto port = static_cast(std::stoul(argv[2])); - const std::size_t count = (argc >= 4) ? std::stoul(argv[3]) : 20; - const int recv_buffer_bytes = (argc >= 5) ? std::stoi(argv[4]) : 0; - return subscribe(port, count, recv_buffer_bytes); + return run_subscribe(argc, argv); } if (argc >= 3 && std::string(argv[1]) == "publish") { - const auto port = static_cast(std::stoul(argv[2])); - const std::uint64_t seed = (argc >= 4) ? std::stoull(argv[3]) : 42; - const std::size_t orders = (argc >= 5) ? std::stoul(argv[4]) : 200; - return publish(port, seed, orders); + return run_publish(argc, argv); } - std::cerr << "usage:\n qsl-mdfeed subscribe [count] [rcvbuf_bytes]\n qsl-mdfeed " - "publish [seed] [orders]\n"; - return 2; + return usage(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 45669f1..9c862f5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -82,6 +82,38 @@ add_test( -D "EXPECT_OUTPUT=orders must be an unsigned integer" -P "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") +# CLI argument hardening: malformed numeric args must print usage and exit non-zero, not throw out +# of main (std::terminate) or silently truncate an out-of-range port. +add_test( + NAME qsl_client_invalid_port_fails + COMMAND ${CMAKE_COMMAND} -D "PROGRAM=$" -D "ARGS=99xyz" -D + "EXPECT_OUTPUT=usage:" -P + "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") + +add_test( + NAME qsl_client_out_of_range_port_fails + COMMAND ${CMAKE_COMMAND} -D "PROGRAM=$" -D "ARGS=70000" -D + "EXPECT_OUTPUT=usage:" -P + "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") + +add_test( + NAME qsl_mdfeed_invalid_count_fails + COMMAND + ${CMAKE_COMMAND} -D "PROGRAM=$" -D "ARGS=subscribe;9009;notanumber" -D + "EXPECT_OUTPUT=usage:" -P "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") + +add_test( + NAME qsl_mdfeed_out_of_range_port_fails + COMMAND ${CMAKE_COMMAND} -D "PROGRAM=$" -D "ARGS=publish;70000" -D + "EXPECT_OUTPUT=usage:" -P + "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") + +add_test( + NAME qsl_export_fixture_invalid_seed_fails + COMMAND ${CMAKE_COMMAND} -D "PROGRAM=$" -D "ARGS=abc" -D + "EXPECT_OUTPUT=usage:" -P + "${CMAKE_CURRENT_LIST_DIR}/cmake/expect_command_failure.cmake") + # Shell unit tests for the shared artifact-publish helper (MAC sanitization + # trailing-whitespace/blank-line trimming). Portable (sed/awk/mktemp); the script # locates the repo via BASH_SOURCE, so it runs correctly by absolute path.