sahvx655-wq opened a new pull request, #3597: URL: https://github.com/apache/brpc/pull/3597
### What problem does this PR solve? Issue Number: N/A Problem Summary: While going through the AMF0 readers in `amf.cpp` after the recent parser hardening there, I noticed the three value readers still handle an object-end marker (`0x09`) in a value position with `CHECK(false)`: `ReadAMFObjectField` for protobuf messages, its `AMFObject` overload, and `ReadAMFArrayItem`. The marker byte is cut straight off the wire, so any RTMP peer reaches these switches: the message variant through the `connect`/`play`/`publish` command objects (`RtmpChunkStream::OnConnect` reads the command object via `ReadAMFObject` at `rtmp_protocol.cpp:2385`), and the `AMFObject`/`AMFArray` variants through `onMetaData`/`onCuePoint` data messages and client-side `onStatus`/`_result` processing. The root cause is that a stray `0x09` is treated as a programming invariant when it is really just malformed input; AMF0 only defines the marker after an empty field name. A field name followed by `0x09` logs at FATAL level with a stack trace on every such message, and aborts the process outright when `-crash_on_fatal_log` is on (CWE-617), dropping every in-flight RPC on the server. Worse, the `AMFObject` and `AMFArray` variants then `break` out of the switch and return true, so with the flag off the malformed document is accepted as if it were valid and the reader carries on misaligned against the field stream. ### What is changed and the side effects? Changed: The three wire-controlled `CHECK(false)` sites become `LOG(ERROR)` plus `return false`, the same handling as the adjacent unsupported-marker cases in the same switches, so the parse fails and every caller already surfaces that as a protocol error. The fourth occurrence, in the `WriteAMFField` serialiser, is not driven by network input and is left alone. Same reasoning as the mcpack2pb hardening in #3576. `RtmpTest.amf_rejects_stray_object_end_marker` covers all three readers. On master the object and array cases fail (the readers return true) and the FATAL trace fires; with the fix all three inputs are rejected. The full `brpc_rtmp_unittest` passes. Side effects: - Performance effects: none - Breaking backward compatibility: no -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
