ssdp_listener_test.cpp is main()+CHECK too, and Catch2 is not in the
tree. The note still claimed it was the one Catch2 suite and that
converting this file was deferred.
ensure_lan_session() can block for a few seconds waiting for CONNACK.
A cancel that arrives in that window must not publish project_file, and
must be reported as a cancel rather than a failed reconnect.
Workspace-only artifacts (INTERESTING_REPOS.md, A1_DIAGNOSTIC_CRIB.md, REMOTE_CAMERA_INVESTIGATION_HANDOFF.md) do not exist in this repo; tags become 'community-survey cross-check #N' and dead links are dropped while keeping every sourced fact.
- maps each of the six mandated notes to its INTERESTING_REPOS.md source row
- citation audit: AGPL facts-only (fence count 6, SIGNED_VS_UNSIGNED absent), both-readings register intact, insertion-only
- read_line() grew ctrl_buf until a newline showed up, and read_reply()
accumulated "NNN-" continuation lines forever. The per-read timeout
does not help while data keeps arriving. Cap a line at 16 KiB and a
reply at 256 KiB and fail the command beyond that.
- The 227 reply was parsed with sscanf("%d"), which is undefined on
out-of-range input, and nothing checked the fields were 0..255.
detail::parse_pasv_reply() now requires six plain decimals in range
and a non-zero port; ftps_parse_test covers it.
Checked against a fake server streaming a newline-less banner and an
endless multi-line 220: both fail after the cap instead of buffering
megabytes.
persist_locked() opened obn.auth.json.tmp with std::ofstream - created
with the umask's mode, typically 0644 - wrote the access and refresh
tokens into it, and only then chmod'ed it to 0600. Another local user
could read the tokens in that window, and a stale .tmp from an older
build kept whatever mode it had.
Pre-create the temp file with open(O_CREAT, 0600) plus fchmod() for the
stale case; rename() carries that mode over, so the chmod after the
fact is gone. umask is process-wide, so not an option inside the
slicer. The chmod on load still tightens files written by old builds.
auth_persist_test checks the result under umask 022 with a
world-readable stale .tmp in place; it fails without the fix.
The opt-in curl trace logged request headers verbatim, including
Authorization: Bearer <access token> - and people attach these logs to
GitHub issues. Mask the values of Authorization, Proxy-Authorization,
Cookie, Set-Cookie, x-amz-/x-oss-security-token and
x-bbl-device-security-sign, keeping the auth scheme word and the value
length so the log stays useful for debugging signatures.
http_redact_test covers the header block shape libcurl passes, case
insensitivity, look-alike names left alone, and degenerate lines.
Agent copied lan_session_, cloud_session_ and discovery_ out as raw
pointers under mu_, unlocked, then used them - while connect_printer(),
disconnect_printer(), disconnect_cloud() or start_discovery(false) could
move the same member out and destroy it. send_message_to_printer() even
does RSA signing in that window. connect_cloud() used cloud_session_
without the lock at all.
One level down the sessions had the same bug: LanSession/CloudSession
disconnect()/stop() reset client_ while publish_json()/publish() and
is_connected() on another thread were still using it. That is the one
ASan catches first in a stress run (publish on one thread, connect/
disconnect on another): heap-use-after-free in mqtt::Client::publish,
freed by LanSession::disconnect.
- The three Agent members and the sessions' client_ are shared_ptr;
callers copy them under the lock and use the copy.
- Whoever takes one out of its member now calls disconnect()/stop()
explicitly (connect_printer's previous session and a racing
replacement included). That joins the network thread, so the last
release - which may now happen on another thread - is a plain destroy
and never lands on the object's own thread.
- mqtt::Client callbacks use a raw pointer to the client that owns them
instead of re-reading client_.
The same stress run is clean after the change, with mqtt_keep_connection
on and off (~1.2M publishes / ~3k reconnect cycles per 8 s run).
- json_lite_test (new): nesting at and past the cap for arrays, objects
and mixed containers; a million-deep payload is rejected, not a stack
overflow; sibling branches give their depth back; as_int() clamps
+-1e300 and +-2^63 and leaves ordinary values alone.
- tunnel_local_test: upload init reply accepts chunk_size 1..65536 KB and
rejects 0, negative, 65537, 2^32 and 1e300; rejects a negative resume
offset.
Both fail if the cap is reverted to 512 or the offset check is removed.
offset is printer-supplied like chunk_size. A negative value wrapped to
~2^64, so the resume-prefix hashing loop read the whole file before
failing with a misleading "read error". Reject it at parse time.
Each level costs about 0.5 KB of stack with GCC, so 512 levels needed
~210-250 KB. Parsing runs on threads we don't size - mosquitto's loop
thread (push_status) and detached std::threads - which get 512 KB by
default on macOS, leaving a thin margin once the callers' frames are
counted. Real Bambu payloads nest in single digits, so 128 still leaves
an order of magnitude of headroom.
The catch-all added in #103 reported FT_EIO unconditionally. If the throw
came after the job had already delivered its result, Studio's callback ran
twice. And a throw mid-transfer can leave the shared TLS session part-way
through a frame, which the next job would reuse because is_connected()
still says yes. Disconnect it and only report if nothing was reported.
ft_tunnel_start_connect() invoked Studio's connect and status callbacks
while holding lan_mu - the same pattern #103 fixed for the upload and
download result paths. On a failed connect FileTransferObject fails its
pending requests, which can reach reset_locked() -> ft_tunnel_shutdown()
and re-lock lan_mu on the same thread.
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>
Cloud MQTT now verifies chain and hostname on every platform, which
breaks behind a TLS-inspecting proxy whose root is not in the vendored
bundle, or once that bundle goes stale. lan_tls_skip_verify (and
OBN_SKIP_TLS_VERIFY) now disables both checks for cloud MQTT too, so
there is an escape hatch without a rebuild. The effective setting is
logged on connect as tls_verify=0/1.
Docs: obn.conf template, README and STATUS describe the wider scope and
no longer claim slicer_base64.cer is used for Windows cloud MQTT.
The Windows no-ca_file branch meant to fall back to OpenSSL's default
verify paths, but mosquitto_tls_set() rejects cafile=capath=NULL with
MOSQ_ERR_INVAL, so connect() always failed there. Turn on
MOSQ_OPT_TLS_USE_OS_CERTS instead (mosquitto enables TLS on that alone)
and only call mosquitto_tls_set() when there is a path to hand it.
With verification on this still fails closed on Windows, since the static
OpenSSL has no default store; with tls_insecure it now connects.
- Linux/macOS take the system trust store, so the ~189 KB bundle was dead
weight in their binaries.
- Register resources/cacert.pem as a configure dependency; it is read by
file(READ) at configure time, so an updated bundle was silently ignored
until a manual reconfigure.
- End every raw-string chunk on a newline instead of a fixed byte offset.
The bundle carries non-ASCII CA names, and with /utf-8 a chunk that
splits a multi-byte sequence gets mangled. The current bundle happens to
be safe; the next refresh might not be.
The header pointed at a nonexistent cmake/EmbedCloudCaBundle.cmake, and
connect_cloud() credited CloudSession::configure() with the verification
flags that actually live in CloudSession::start().
* fix: verify the cloud MQTT TLS certificate on Windows instead of skipping it
connect_cloud() handed CloudSession the BBL slicer cert bundle Studio
passes via set_cert_file() as the cloud MQTT trust anchor, purely so
mosquitto_tls_set() wouldn't reject a null cafile (vcpkg's static
OpenSSL ships no default trust store on Windows). Since that bundle's
roots don't actually sign *.bambulab.com, CloudSession then set
tls_skip_chain_verify=true and called mosquitto_tls_insecure_set(true)
-- disabling *both* certificate-chain and hostname verification for
the cloud connection, unconditionally, whenever a cert bundle path was
configured (i.e. on every normal Windows run).
That's a real gap: with verification off, an active MITM (rogue AP,
ARP/DNS spoofing, a malicious proxy) can terminate the TLS session
itself and read the plaintext MQTT password, which is the user's live
cloud bearer token -- enough to fully impersonate them against the
real Bambu cloud API until the token expires. The comment justifying
the skip ("MITM gets opaque traffic but no usable credentials") does
not hold once verification is disabled.
Checked what's actually needed: *.bambulab.com serves an ordinary,
publicly-trusted certificate --
$ openssl s_client -connect us.mqtt.bambulab.com:8883 \
-CAfile cacert.pem
...
Verify return code: 0 (ok)
against curl's official cacert.pem (a standard Mozilla-derived bundle,
the same kind curl's own Windows build already uses successfully for
this project's HTTPS calls to api.bambulab.com). It chains to DigiCert
Global Root G2 -- nothing Bambu-specific required.
So: vendor that bundle (resources/cacert.pem, compiled in via
cmake/cloud_ca_bundle_data.cpp.in so the plugin doesn't depend on a
companion file being installed alongside it), write it to
<config_dir>/obn_cloud_ca.pem on Windows, and hand mosquitto that
instead of Studio's BBL bundle. CloudSession::start() no longer skips
chain or hostname verification for the cloud path -- the CA bundle it
receives is now one that actually validates the real chain, so the
normal cfg.tls_insecure = false already at the top of the function is
correct and sufficient.
Linux/macOS are unaffected (unchanged: empty ca_file falls through to
mqtt_client's distro-trust-store search, which already works).
* fix: split the embedded CA bundle into chunks (MSVC C2026)
MSVC caps a single string literal at 65535 bytes; the ~189 KB vendored
bundle blew past that as one big raw string. Split it into ~8000-byte
chunks emitted as adjacent raw-string literals -- the compiler
concatenates those into one string constant at compile time with no
limit on the total length. g++'s earlier -fsyntax-only check didn't
catch this since GCC has no such per-literal limit; caught by an
actual MSVC build.
---------
Co-authored-by: Fernando Marino <f.marino@rheagroup.com>
prefer_rtsp treated an open CTRL port as a camera and handed Studio a
LAN URL with no RTSP hint, so BambuSource spoke MJPEG. A printer that
reports rtsp_url "disable" now falls through to TUTK instead.
Alexey Cluster