Hostile-input test banks: JSONTestSuite corpus + HTTP framing

runtime/BUILD.bazel

@@ -145,6 +145,18 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "socket_transport_hostile_test",
+    size = "small",
+    srcs = ["tests/http/socket_transport_hostile_test.cc"],
+    copts = COPTS,
+    tags = ["requires-network"],
+    deps = [
+        ":http",
+        "@googletest//:gtest_main",
+    ],
+)
+
 # Production server transport (ADR-0006). Separate target so consumers that
 # only need the client, loopback, or test transports don't pull in Boost.
 cc_library(
@@ -352,6 +364,51 @@ cc_test(
     ],
 )
 
+# Bakes the vendored JSONTestSuite filenames into a source the conformance test
+# links: runfiles directory enumeration isn't portable, so the parametrized
+# case list comes from the checked-in files at build time.
+genrule(
+    name = "json_conformance_index",
+    srcs = glob(["tests/json/jsontestsuite/test_parsing/*.json"]),
+    outs = ["json_conformance_index.cc"],
+    cmd = "\n".join([
+        "{",
+        "  echo '#include <string>'",
+        "  echo '#include <vector>'",
+        "  echo 'namespace smithy::json {'",
+        "  echo 'std::vector<std::string> CorpusFiles() {'",
+        "  echo '  return {'",
+        "  for f in $(SRCS); do echo \"    \\\"$$(basename $$f)\\\",\"; done",
+        "  echo '  };'",
+        "  echo '}'",
+        "  echo '}  // namespace smithy::json'",
+        "} > $@",
+    ]),
+)
+
+cc_test(
+    name = "json_conformance_test",
+    size = "small",
+    srcs = [
+        "tests/json/json_conformance_test.cc",
+        ":json_conformance_index",
+    ],
+    copts = COPTS,
+    data = glob(["tests/json/jsontestsuite/test_parsing/*.json"]),
+    # The JSON parser is platform-independent; run the corpus on POSIX and skip
+    # the runfiles/genrule-bash portability surface on Windows for no added
+    # coverage.
+    target_compatible_with = select({
+        "@platforms//os:windows": ["@platforms//:incompatible"],
+        "//conditions:default": [],
+    }),
+    deps = [
+        ":json",
+        "@bazel_tools//tools/cpp/runfiles",
+        "@googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "cbor_test",
     size = "small",

runtime/src/http/socket_transport.cc

@@ -130,6 +130,17 @@ Outcome<ParsedMessage> ReadMessage(SocketFd fd, bool body_until_eof) {
     header_block.remove_prefix(eol + 2);
   }
 
+  // Reject ambiguous or unsupported framing before trusting a body length —
+  // the classic request-smuggling desync vectors. Neither transport direction
+  // implements chunked transfer, and conflicting content-lengths let a proxy
+  // and this server disagree on message boundaries.
+  if (message.headers.GetAll("content-length").size() > 1) {
+    return Error::Transport("http: conflicting content-length");
+  }
+  if (message.headers.Has("transfer-encoding")) {
+    return Error::Transport("http: transfer-encoding is not supported");
+  }
+
   message.body = buffer.substr(header_end + 4);
   if (const auto length_text = message.headers.Get("content-length")) {
     char* end = nullptr;
@@ -292,6 +303,11 @@ void SocketHttpServer::AcceptLoop() {
       response = HttpResponse{400, {}, message.error().message()};
     }
 
+    // The transport is authoritative for framing, so drop any copies a handler
+    // set — otherwise they are emitted twice (a duplicate content-length is
+    // exactly the smuggling vector a strict peer now rejects).
+    response.headers.Remove("content-length");
+    response.headers.Remove("connection");
     std::string wire = "HTTP/1.1 " + std::to_string(response.status) + " \r\n";
     wire += "content-length: " + std::to_string(response.body.size()) + "\r\n";
     wire += "connection: close\r\n";

runtime/tests/http/socket_transport_hostile_test.cc

@@ -0,0 +1,186 @@
+// Hostile HTTP/1.1 framing against SocketHttpServer's hand-rolled parser
+// (ReadMessage). SocketHttpClient only ever emits well-formed requests, so
+// these drive raw bytes down a socket to exercise the paths a real attacker
+// controls: request-smuggling framing, malformed content-lengths, header
+// floods, truncation. The invariant is that the server never crashes, never
+// hangs, and never mis-parses ambiguous framing as a successful request.
+//
+// POSIX only: the parser under test is platform-independent, and a portable
+// raw-socket client would drag in winsock scaffolding for no extra coverage.
+
+#ifndef _WIN32
+
+#include <arpa/inet.h>
+#include <gtest/gtest.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <unistd.h>
+
+#include <optional>
+#include <string>
+
+#include "smithy/http/socket_transport.h"
+
+namespace smithy::http {
+namespace {
+
+// Sends raw bytes to 127.0.0.1:port, half-closes so the server sees EOF (no
+// waiting on the read timeout), and returns whatever the server wrote back —
+// or nullopt if the connection could not be made. An empty string means the
+// server closed without a response.
+std::optional<std::string> RawExchange(int port, const std::string& request) {
+  const int fd = ::socket(AF_INET, SOCK_STREAM, 0);
+  if (fd < 0) return std::nullopt;
+  sockaddr_in addr{};
+  addr.sin_family = AF_INET;
+  addr.sin_port = htons(static_cast<std::uint16_t>(port));
+  addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+  // Bound the whole exchange so a parser bug surfaces as a test timeout on
+  // this socket, not a hung test binary.
+  timeval tv{};
+  tv.tv_sec = 5;
+  setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+  setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv));
+  if (::connect(fd, reinterpret_cast<sockaddr*>(&addr), sizeof(addr)) != 0) {
+    ::close(fd);
+    return std::nullopt;
+  }
+  const char* data = request.data();
+  std::size_t remaining = request.size();
+  while (remaining > 0) {
+    const auto sent = ::send(fd, data, remaining, MSG_NOSIGNAL);
+    if (sent <= 0) break;
+    data += sent;
+    remaining -= static_cast<std::size_t>(sent);
+  }
+  ::shutdown(fd, SHUT_WR);  // signal end-of-request
+
+  std::string response;
+  char buffer[4096];
+  while (true) {
+    const auto got = ::recv(fd, buffer, sizeof(buffer), 0);
+    if (got <= 0) break;
+    response.append(buffer, static_cast<std::size_t>(got));
+  }
+  ::close(fd);
+  return response;
+}
+
+// Parses the status code from an "HTTP/1.1 NNN ..." response, or nullopt if the
+// server sent nothing / something unparseable (a rejection, which is fine).
+std::optional<int> StatusOf(const std::optional<std::string>& response) {
+  if (!response || response->rfind("HTTP/", 0) != 0) return std::nullopt;
+  const auto space = response->find(' ');
+  if (space == std::string::npos) return std::nullopt;
+  return std::atoi(response->c_str() + space + 1);
+}
+
+class HostileFramingTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    // A 200-returning handler: if hostile framing were mis-parsed as a valid
+    // request, we would observe 200 — which every case below forbids.
+    ASSERT_TRUE(server_.Start([](const HttpRequest&) { return HttpResponse{200, {}, "ok"}; }).ok());
+  }
+  void TearDown() override { server_.Stop(); }
+
+  SocketHttpServer server_;
+};
+
+TEST_F(HostileFramingTest, RejectsConflictingContentLength) {
+  // The body exactly matches the first content-length, so nothing but the
+  // explicit duplicate-header rejection stops this: a proxy honoring the
+  // second length and this server honoring the first is the smuggling desync.
+  const auto response = RawExchange(
+      server_.port(),
+      "POST / HTTP/1.1\r\nhost: x\r\ncontent-length: 5\r\ncontent-length: 6\r\n\r\nhello");
+  const auto status = StatusOf(response);
+  // Ambiguous framing must never be accepted as a valid request.
+  EXPECT_NE(status.value_or(400), 200);
+}
+
+TEST_F(HostileFramingTest, RejectsTransferEncoding) {
+  const auto response = RawExchange(
+      server_.port(),
+      "POST / HTTP/1.1\r\nhost: x\r\ntransfer-encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n");
+  EXPECT_NE(StatusOf(response).value_or(400), 200);
+}
+
+TEST_F(HostileFramingTest, RejectsNegativeContentLength) {
+  const auto response =
+      RawExchange(server_.port(), "POST / HTTP/1.1\r\nhost: x\r\ncontent-length: -1\r\n\r\n");
+  EXPECT_NE(StatusOf(response).value_or(400), 200);
+}
+
+TEST_F(HostileFramingTest, RejectsNonNumericContentLength) {
+  const auto response =
+      RawExchange(server_.port(), "POST / HTTP/1.1\r\nhost: x\r\ncontent-length: abc\r\n\r\n");
+  EXPECT_NE(StatusOf(response).value_or(400), 200);
+}
+
+TEST_F(HostileFramingTest, RejectsOverflowingContentLength) {
+  const auto response =
+      RawExchange(server_.port(),
+                  "POST / HTTP/1.1\r\nhost: x\r\ncontent-length: 999999999999999999999999\r\n\r\n");
+  EXPECT_NE(StatusOf(response).value_or(400), 200);
+}
+
+TEST_F(HostileFramingTest, SurvivesHeaderFlood) {
+  // Thousands of headers, kept just under the 64 KiB header cap. Must not
+  // crash or hang; whether it 200s or 400s is not the point.
+  std::string request = "GET / HTTP/1.1\r\nhost: x\r\n";
+  for (int i = 0; i < 2000 && request.size() < 60 * 1024; ++i) {
+    request += "x-pad-" + std::to_string(i) + ": v\r\n";
+  }
+  request += "\r\n";
+  const auto response = RawExchange(server_.port(), request);
+  // Reached here without the server dying — that is the assertion. A response
+  // is expected but its code is unconstrained.
+  EXPECT_TRUE(response.has_value());
+}
+
+TEST_F(HostileFramingTest, HandlesTruncatedHeadersWithoutHanging) {
+  // Headers that never terminate, then EOF. The server must give up (bounded
+  // by the half-close) rather than block forever.
+  const auto response = RawExchange(server_.port(), "GET / HTTP/1.1\r\nhost: x\r\nx-partial: ");
+  EXPECT_NE(StatusOf(response).value_or(400), 200);
+}
+
+TEST_F(HostileFramingTest, StillServesAWellFormedRequestAfterward) {
+  // Sanity: the hardening didn't break the normal path.
+  const auto response =
+      RawExchange(server_.port(), "GET / HTTP/1.1\r\nhost: x\r\ncontent-length: 0\r\n\r\n");
+  EXPECT_EQ(StatusOf(response).value_or(0), 200);
+}
+
+TEST(HostileFramingResponseTest, ServerEmitsExactlyOneContentLength) {
+  // Regression: a handler that sets its own content-length (generated servers
+  // do this for payload responses) must not cause the transport to emit a
+  // second one — a duplicate a strict client now rejects.
+  SocketHttpServer server;
+  ASSERT_TRUE(server
+                  .Start([](const HttpRequest&) {
+                    HttpResponse response{200, {}, "body"};
+                    response.headers.Set("content-length", "4");
+                    return response;
+                  })
+                  .ok());
+  const auto response =
+      RawExchange(server.port(), "GET / HTTP/1.1\r\nhost: x\r\ncontent-length: 0\r\n\r\n");
+  server.Stop();
+  ASSERT_TRUE(response.has_value());
+  // Count content-length header lines in the response head.
+  int count = 0;
+  std::size_t pos = 0;
+  const std::string needle = "content-length:";
+  while ((pos = response->find(needle, pos)) != std::string::npos) {
+    ++count;
+    pos += needle.size();
+  }
+  EXPECT_EQ(count, 1) << *response;
+}
+
+}  // namespace
+}  // namespace smithy::http
+
+#endif  // _WIN32

runtime/tests/json/json_conformance_test.cc

@@ -0,0 +1,91 @@
+// Runs the vendored JSONTestSuite parsing corpus (nst/JSONTestSuite, see
+// tests/json/jsontestsuite/PROVENANCE.md) through smithy::json::Decode. The
+// suite is the canonical bank for "does the parser agree with RFC 8259 and
+// never crash on hostile input" — the class of bug that produced the
+// nesting-depth stack overflow this test was added with.
+//
+// Invariant for every file: Decode returns, never crashes or hangs. On top of
+// that, y_ files must be accepted and n_ files rejected (with a documented
+// allowlist); i_ files are implementation-defined, so no-crash is the only
+// requirement.
+
+#include <gtest/gtest.h>
+
+#include <fstream>
+#include <set>
+#include <sstream>
+#include <string>
+#include <vector>
+
+#include "smithy/json/json.h"
+#include "tools/cpp/runfiles/runfiles.h"
+
+namespace smithy::json {
+
+// Defined in the generated json_conformance_index.cc (a genrule lists the
+// vendored corpus filenames): runfiles directory enumeration is not portable,
+// so the filenames are baked in at build time from the checked-in files.
+std::vector<std::string> CorpusFiles();
+
+namespace {
+
+using bazel::tools::cpp::runfiles::Runfiles;
+
+// n_ cases the nlohmann backend accepts. Documented, not fixed — see
+// PROVENANCE.md. "123\0": a number then a NUL byte, tolerated as trailing
+// whitespace.
+const std::set<std::string>& AcceptedNegatives() {
+  static const std::set<std::string> kAllow = {"n_multidigit_number_then_00.json"};
+  return kAllow;
+}
+
+std::string ReadFile(const std::string& path) {
+  std::ifstream in(path, std::ios::binary);
+  std::ostringstream out;
+  out << in.rdbuf();
+  return out.str();
+}
+
+class JsonConformanceTest : public testing::TestWithParam<std::string> {};
+
+TEST_P(JsonConformanceTest, MatchesRfc8259AndNeverCrashes) {
+  std::string error;
+  std::unique_ptr<Runfiles> runfiles(Runfiles::CreateForTest(&error));
+  ASSERT_NE(runfiles, nullptr) << error;
+
+  const std::string name = GetParam();
+  const std::string path =
+      runfiles->Rlocation("_main/runtime/tests/json/jsontestsuite/test_parsing/" + name);
+  const std::string content = ReadFile(path);
+
+  // The load-bearing assertion: this call returns rather than crashing or
+  // hanging, whatever the verdict.
+  const auto decoded = Decode(content);
+
+  switch (name[0]) {
+    case 'y':
+      EXPECT_TRUE(decoded.ok()) << "valid JSON rejected: " << decoded.error().message();
+      break;
+    case 'n':
+      if (AcceptedNegatives().count(name) == 0) {
+        EXPECT_FALSE(decoded.ok()) << "invalid JSON accepted";
+      }
+      break;
+    default:
+      break;  // i_: implementation-defined, no-crash already asserted.
+  }
+}
+
+INSTANTIATE_TEST_SUITE_P(JSONTestSuite, JsonConformanceTest, testing::ValuesIn(CorpusFiles()),
+                         [](const testing::TestParamInfo<std::string>& info) {
+                           // Distinct filenames can sanitize to the same
+                           // identifier (1.0e- vs 1.0e+), so prefix the index.
+                           std::string id = std::to_string(info.index) + "_" + info.param;
+                           for (char& c : id) {
+                             if (!std::isalnum(static_cast<unsigned char>(c))) c = '_';
+                           }
+                           return id;
+                         });
+
+}  // namespace
+}  // namespace smithy::json