fix: harden JSON parsing and the FT job thread against malicious wire data (#103)

Three related bugs, all reachable from untrusted printer/cloud input
since the plugin trusts wire data it hasn't fully validated:

1. json_lite.cpp: Parser::parse_value() recursed once per nested {}/[]
   with no depth limit. A deeply-nested JSON payload (e.g. thousands of
   nested '[') exhausts the stack and crashes the host process, since
   this plugin runs in-process via dlopen, not sandboxed. It's reachable
   from raw MQTT push_status payloads (agent.cpp's update_fw_state) and
   from any HTTP/cloud response body. Cap nesting at 512 levels.

2. json_lite.hpp: Value::as_int() did a bare
   static_cast<int64_t>(double), which is undefined behavior when the
   number is NaN or outside int64_t's range - both are things wire JSON
   can freely contain. Clamp instead.

3. tunnel_local.cpp: parse_upload_init_reply() read the printer's
   FILE_UPLOAD-init reply.chunk_size with no upper bound. That value
   directly drives `buffer_size = chunk_size_kb * 1024` and
   `std::vector<char> buffer(buffer_size)` in tunnel_upload.cpp. A
   crafted reply (e.g. chunk_size: -1) requests a multi-terabyte
   allocation, throwing std::bad_alloc/std::length_error. That runs on
   the detached job thread spawned by abi_ft.cpp::spawn_job() with no
   exception handling, so the exception escapes and calls
   std::terminate() - the whole host process dies from one small
   malicious reply. Reject anything above 64 MiB (observed real
   firmware reports low hundreds of KB).

Also, as defense in depth for #3 and any other not-yet-found exception
source on that same thread: wrap spawn_job()'s job dispatch in
try/catch and report FT_EIO instead of letting any exception reach
std::terminate().

Separately, abi_ft.cpp's connect-failure error path called
deliver_result() (and thus the host's result callback, synchronously)
while still holding FT_Tunnel::lan_mu. Studio's FileTransferObject
calls ft_tunnel_shutdown() from that callback for one-shot mem
downloads, which re-locks the same non-recursive mutex on the same
thread -> deadlock. Move the error delivery outside the locked scope,
matching the pattern the success path already used.

Verified: all four files pass `g++ -std=c++17 -fsyntax-only`. The
self-contained json_lite fix is additionally covered by a standalone
test exercising deep nesting (now rejected instead of crashing),
normal/moderately-nested JSON (still parses), and as_int() clamping on
NaN/out-of-range doubles - all pass. The abi_ft.cpp/tunnel_local.cpp
fixes could not be exercised end-to-end (no Bambu printer available,
and the full build needs MSVC v142 + vcpkg), so please review the
locking/allocation-bound logic carefully before merging.

Co-authored-by: Fernando Marino <f.marino@rheagroup.com>

include/obn/json_lite.hpp

@@ -13,7 +13,9 @@
 //   * Every last JSON oddity (UTF-16 surrogate pairs in \uXXXX escapes
 //     are handled, but we don't renormalise).
 
+#include <cmath>
 #include <cstdint>
+#include <limits>
 #include <map>
 #include <memory>
 #include <optional>
@@ -54,7 +56,18 @@ public:
 
     bool        as_bool(bool def = false)   const { return is_bool() ? boolean_ : def; }
     double      as_number(double def = 0)   const { return is_number() ? number_ : def; }
-    std::int64_t as_int(std::int64_t d = 0) const { return is_number() ? static_cast<std::int64_t>(number_) : d; }
+    // static_cast<int64_t> of a double outside [INT64_MIN, INT64_MAX] (or NaN)
+    // is undefined behavior. Wire data (printer/cloud JSON) is untrusted, so
+    // clamp instead of trusting the field to be in range.
+    std::int64_t as_int(std::int64_t d = 0) const {
+        if (!is_number()) return d;
+        if (std::isnan(number_)) return d;
+        if (number_ >= static_cast<double>(std::numeric_limits<std::int64_t>::max()))
+            return std::numeric_limits<std::int64_t>::max();
+        if (number_ <= static_cast<double>(std::numeric_limits<std::int64_t>::min()))
+            return std::numeric_limits<std::int64_t>::min();
+        return static_cast<std::int64_t>(number_);
+    }
     const std::string& as_string() const {
         static const std::string empty;
         return is_string() ? string_ : empty;

src/abi_ft.cpp

@@ -22,6 +22,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <exception>
 #include <memory>
 #include <mutex>
 #include <random>
@@ -409,18 +410,24 @@ void run_upload_job(FT_Tunnel* t, FT_Job* j)
     };
 
     obn::tunnel_upload::UploadOutcome outcome;
+    std::string connect_err;
     {
         std::lock_guard<std::mutex> lk(t->lan_mu);
-        if (std::string err = connect_lan_tunnel(t); !err.empty()) {
-            OBN_WARN("ft: upload: connect failed: %s", err.c_str());
-            deliver_result(j, FT_EIO, 0, {});
-            return;
+        connect_err = connect_lan_tunnel(t);
+        if (connect_err.empty()) {
+            outcome = t->conn->upload(req, cb);
         }
-        outcome = t->conn->upload(req, cb);
     }
 
     // Never call result_cb under lan_mu — Studio's FileTransferObject invokes
-    // ft_tunnel_shutdown() from the result callback (one-shot mem download).
+    // ft_tunnel_shutdown() from the result callback (one-shot mem download),
+    // which re-locks lan_mu; calling it while still holding the lock above
+    // would deadlock.
+    if (!connect_err.empty()) {
+        OBN_WARN("ft: upload: connect failed: %s", connect_err.c_str());
+        deliver_result(j, FT_EIO, 0, {});
+        return;
+    }
     if (outcome.ok) {
         deliver_result(j, 0, outcome.wire_result, {});
         return;
@@ -457,16 +464,21 @@ void run_download_job(FT_Tunnel* t, FT_Job* j)
     };
 
     obn::tunnel_upload::DownloadOutcome outcome;
+    std::string connect_err;
     {
         std::lock_guard<std::mutex> lk(t->lan_mu);
-        if (std::string err = connect_lan_tunnel(t); !err.empty()) {
-            OBN_WARN("ft: download: connect failed: %s", err.c_str());
-            deliver_result(j, FT_EIO, 0, {});
-            return;
+        connect_err = connect_lan_tunnel(t);
+        if (connect_err.empty()) {
+            outcome = t->conn->download(req, cb);
         }
-        outcome = t->conn->download(req, cb);
     }
 
+    // Never call result_cb under lan_mu — see run_upload_job().
+    if (!connect_err.empty()) {
+        OBN_WARN("ft: download: connect failed: %s", connect_err.c_str());
+        deliver_result(j, FT_EIO, 0, {});
+        return;
+    }
     if (outcome.ok) {
         OBN_DEBUG("ft: download ok path=%s bytes=%zu reply=%.200s",
                  req.path.c_str(), outcome.data.size(),
@@ -489,15 +501,27 @@ void spawn_job(FT_Tunnel* t, FT_Job* j)
     retain(t);
     retain(j);
     std::thread([t, j] {
-        switch (j->cmd_type) {
-        case kCmdTypeMediaAbility: run_ability_job(t, j); break;
-        case kCmdTypeUpload:       run_upload_job(t, j);  break;
-        case kCmdTypeDownload:     run_download_job(t, j); break;
-        default:
-            OBN_WARN("ft: unknown cmd_type=%d (raw=%.200s)",
-                     j->cmd_type, j->raw_params.c_str());
+        // A job thread runs detached; an exception escaping it terminates the
+        // whole host process (Studio/Orca), not just this job. Wire data
+        // (printer replies) feeds these code paths, so treat any unexpected
+        // exception as a failed job rather than letting it propagate.
+        try {
+            switch (j->cmd_type) {
+            case kCmdTypeMediaAbility: run_ability_job(t, j); break;
+            case kCmdTypeUpload:       run_upload_job(t, j);  break;
+            case kCmdTypeDownload:     run_download_job(t, j); break;
+            default:
+                OBN_WARN("ft: unknown cmd_type=%d (raw=%.200s)",
+                         j->cmd_type, j->raw_params.c_str());
+                deliver_result(j, FT_EIO, 0, {});
+                break;
+            }
+        } catch (const std::exception& e) {
+            OBN_WARN("ft: job cmd_type=%d threw: %s", j->cmd_type, e.what());
+            deliver_result(j, FT_EIO, 0, {});
+        } catch (...) {
+            OBN_WARN("ft: job cmd_type=%d threw non-std::exception", j->cmd_type);
             deliver_result(j, FT_EIO, 0, {});
-            break;
         }
         release(j);
         release(t);

src/json_lite.cpp

@@ -9,10 +9,16 @@ namespace obn::json {
 
 namespace {
 
+// Nested {}/[] recurse one C++ stack frame per level with no other bound;
+// wire data (MQTT push_status, HTTP/cloud responses) can be arbitrarily
+// deeply nested, so cap it well short of a real stack overflow.
+constexpr int kMaxNestingDepth = 512;
+
 struct Parser {
     const char* p;
     const char* end;
     std::string err;
+    int depth = 0;
 
     bool at_end() const { return p >= end; }
 
@@ -152,31 +158,42 @@ struct Parser {
             out = Value(std::move(s));
             return true;
         }
-        if (c == '{') {
-            ++p;
-            Object obj;
-            skip_ws();
-            if (p < end && *p == '}') { ++p; out = Value(std::move(obj)); return true; }
-            while (true) {
-                skip_ws();
-                std::string key;
-                if (!parse_string(key)) return false;
-                skip_ws();
-                if (p >= end || *p != ':') { err = "expected ':'"; return false; }
-                ++p;
-                Value v;
-                if (!parse_value(v)) return false;
-                obj.emplace(std::move(key), std::move(v));
-                skip_ws();
-                if (p < end && *p == ',') { ++p; continue; }
-                if (p < end && *p == '}') { ++p; break; }
-                err = "expected ',' or '}'";
+        if (c == '{' || c == '[') {
+            if (depth >= kMaxNestingDepth) {
+                err = "max nesting depth exceeded";
                 return false;
             }
-            out = Value(std::move(obj));
-            return true;
-        }
-        if (c == '[') {
+            ++depth;
+            struct DepthGuard {
+                int& d;
+                ~DepthGuard() { --d; }
+            } depth_guard{depth};
+
+            if (c == '{') {
+                ++p;
+                Object obj;
+                skip_ws();
+                if (p < end && *p == '}') { ++p; out = Value(std::move(obj)); return true; }
+                while (true) {
+                    skip_ws();
+                    std::string key;
+                    if (!parse_string(key)) return false;
+                    skip_ws();
+                    if (p >= end || *p != ':') { err = "expected ':'"; return false; }
+                    ++p;
+                    Value v;
+                    if (!parse_value(v)) return false;
+                    obj.emplace(std::move(key), std::move(v));
+                    skip_ws();
+                    if (p < end && *p == ',') { ++p; continue; }
+                    if (p < end && *p == '}') { ++p; break; }
+                    err = "expected ',' or '}'";
+                    return false;
+                }
+                out = Value(std::move(obj));
+                return true;
+            }
+            // c == '['
             ++p;
             Array arr;
             skip_ws();

src/tunnel_local.cpp

@@ -366,9 +366,16 @@ bool parse_upload_init_reply(const std::string& wire_json,
     auto reply = root->find("reply");
     if (!reply.is_object()) return false;
 
-    const std::uint32_t kb =
-        static_cast<std::uint32_t>(reply.find("chunk_size").as_int(0));
-    if (!kb) return false;
+    // Observed real firmware reports chunk_size in the low hundreds (e.g. 255
+    // on a P2S, see research/06.04-port-6000.md). This field drives a
+    // buffer_size = chunk_size_kb * 1024 allocation in the upload path, so a
+    // malicious/corrupted reply must not be allowed to request a
+    // multi-gigabyte buffer. 64 MiB is generously above anything real
+    // firmware sends while keeping the worst case allocation bounded.
+    constexpr std::uint32_t kMaxChunkSizeKb = 64 * 1024;
+    const std::int64_t raw_kb = reply.find("chunk_size").as_int(0);
+    if (raw_kb <= 0 || raw_kb > kMaxChunkSizeKb) return false;
+    const std::uint32_t kb = static_cast<std::uint32_t>(raw_kb);
 
     if (chunk_size_kb) *chunk_size_kb = kb;
     if (offset) {