From 6b16281ab0fc8f10adbc0e3196ba9a4e7a84f15e Mon Sep 17 00:00:00 2001 From: YuLi Date: Mon, 27 Jul 2026 07:56:18 -0700 Subject: [PATCH] fix: limit AMF decoder nesting depth (#4787) ### Motivation - Prevent unbounded recursive-descent parsing in the AMF decoder that allowed deeply nested AMF objects/arrays to exhaust the process stack and cause a pre-auth remote DoS. ### Description - Add an RAII depth guard `AMFDecoder::DepthGuard` and a per-decoder `depth` counter with `kMaxDepth = 64` to `AMFDecoder` in `src/Rtmp/amf.h`/`src/Rtmp/amf.cpp`. - Instrument `load_object()`, `load_ecma()`, and `load_arr()` to create a `DepthGuard` at entry so nesting is incremented and reliably decremented on return or exception, and throw `std::runtime_error("Maximum AMF nesting depth exceeded")` when the limit is exceeded. - Add `tests/test_amf.cpp` to assert that a 64-level nested AMF container is accepted and a 65-level nested container is rejected with the expected error. - Keep non-nested parsing behavior unchanged; the guard only enforces a maximum container nesting depth. ### Testing - Configured the project with tests enabled using `cmake -S . -B build -DENABLE_TESTS=ON`, which generated the `test_amf` target successfully. - Built the test target with `cmake --build build --target test_amf` and performed smoke compiles of modified sources, with `src/Rtmp/amf.cpp` compiling to object (`/tmp/amf.o`) successfully. - Compiled the new test source with `c++ -std=gnu++11 -c tests/test_amf.cpp` successfully and verified `git diff --cached --check` passed. ------ [Codex Task](https://chatgpt.com/codex/cloud/tasks/task_e_6a63c9326b9883209c857c0f4fa5d0cc) --- src/Rtmp/amf.cpp | 17 ++++++- src/Rtmp/amf.h | 16 +++++++ tests/test_amf.cpp | 115 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 tests/test_amf.cpp diff --git a/src/Rtmp/amf.cpp b/src/Rtmp/amf.cpp index b93fa2ce..2cb4c994 100644 --- a/src/Rtmp/amf.cpp +++ b/src/Rtmp/amf.cpp @@ -452,6 +452,17 @@ const std::string& AMFEncoder::data() const { //////////////////Decoder////////////////// +AMFDecoder::DepthGuard::DepthGuard(AMFDecoder &decoder) : _decoder(decoder) { + if (_decoder.depth >= AMFDecoder::kMaxDepth) { + throw std::runtime_error("Maximum AMF nesting depth exceeded"); + } + ++_decoder.depth; +} + +AMFDecoder::DepthGuard::~DepthGuard() { + --_decoder.depth; +} + uint8_t AMFDecoder::front() { if (pos >= buf.size()) { throw std::runtime_error("Not enough data"); @@ -625,6 +636,7 @@ std::string AMFDecoder::load_key() { } AMFValue AMFDecoder::load_object() { + DepthGuard depth_guard(*this); AMFValue object(AMF_OBJECT); if (pop_front() != AMF0_OBJECT) { throw std::runtime_error("Expected an object"); @@ -643,6 +655,7 @@ AMFValue AMFDecoder::load_object() { } AMFValue AMFDecoder::load_ecma() { + DepthGuard depth_guard(*this); /* ECMA array is the same as object, with 4 extra zero bytes */ AMFValue object(AMF_ECMA_ARRAY); if (pop_front() != AMF0_ECMA_ARRAY) { @@ -665,7 +678,8 @@ AMFValue AMFDecoder::load_ecma() { return object; } AMFValue AMFDecoder::load_arr() { - /* ECMA array is the same as object, with 4 extra zero bytes */ + DepthGuard depth_guard(*this); + /* Strict array is a count-prefixed list of AMF values. */ AMFValue object(AMF_STRICT_ARRAY); if (pop_front() != AMF0_STRICT_ARRAY) { throw std::runtime_error("Expected an STRICT array"); @@ -689,4 +703,3 @@ AMFValue AMFDecoder::load_arr() { AMFDecoder::AMFDecoder(const BufferLikeString &buf_in, size_t pos_in, int version_in) : buf(buf_in), pos(pos_in), version(version_in) { } - diff --git a/src/Rtmp/amf.h b/src/Rtmp/amf.h index c3f7ed29..9130451d 100644 --- a/src/Rtmp/amf.h +++ b/src/Rtmp/amf.h @@ -89,6 +89,19 @@ public: TP load(); private: + class DepthGuard { + public: + explicit DepthGuard(AMFDecoder &decoder); + ~DepthGuard(); + DepthGuard(const DepthGuard &) = delete; + DepthGuard(DepthGuard &&) = delete; + DepthGuard &operator=(const DepthGuard &) = delete; + DepthGuard &operator=(DepthGuard &&) = delete; + + private: + AMFDecoder &_decoder; + }; + std::string load_key(); AMFValue load_object(); AMFValue load_ecma(); @@ -100,6 +113,9 @@ private: const toolkit::BufferLikeString &buf; size_t pos; int version; + size_t depth = 0; + + static constexpr size_t kMaxDepth = 64; }; class AMFEncoder { diff --git a/tests/test_amf.cpp b/tests/test_amf.cpp new file mode 100644 index 00000000..48bfbcc1 --- /dev/null +++ b/tests/test_amf.cpp @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2016-present The ZLMediaKit project authors. All Rights Reserved. + * + * This file is part of ZLMediaKit(https://github.com/ZLMediaKit/ZLMediaKit). + * + * Use of this source code is governed by MIT-like license that can be found in the + * LICENSE file in the root of the source tree. All contributing project authors + * may be found in the AUTHORS file in the root of the source tree. + */ + +#include +#include +#include + +#include "Network/Buffer.h" +#include "Rtmp/amf.h" + +using namespace std; +using namespace toolkit; + +enum class ContainerType { + Object, + EcmaArray, + StrictArray, +}; + +static const char *container_name(ContainerType type) { + switch (type) { + case ContainerType::Object: + return "object"; + case ContainerType::EcmaArray: + return "ECMA array"; + case ContainerType::StrictArray: + return "strict array"; + } + return "unknown"; +} + +static AMFType container_amf_type(ContainerType type) { + switch (type) { + case ContainerType::Object: + return AMF_OBJECT; + case ContainerType::EcmaArray: + return AMF_ECMA_ARRAY; + case ContainerType::StrictArray: + return AMF_STRICT_ARRAY; + } + return AMF_UNDEFINED; +} + +static string make_nested_container(ContainerType type, size_t depth) { + string result; + for (size_t i = 0; i < depth; ++i) { + switch (type) { + case ContainerType::Object: + result.append("\x03\x00\x01x", 4); + break; + case ContainerType::EcmaArray: + result.append("\x08\x00\x00\x00\x01\x00\x01x", 8); + break; + case ContainerType::StrictArray: + result.append("\x0A\x00\x00\x00\x01", 5); + break; + } + } + result.append("\x05", 1); + if (type != ContainerType::StrictArray) { + for (size_t i = 0; i < depth; ++i) { + result.append("\x00\x00\x09", 3); + } + } + return result; +} + +static bool accepts_nesting_limit(ContainerType type) { + BufferLikeString input(make_nested_container(type, 64)); + AMFDecoder decoder(input, 0); + try { + return decoder.load().type() == container_amf_type(type); + } catch (const runtime_error &) { + return false; + } +} + +static bool rejects_excessive_nesting(ContainerType type) { + BufferLikeString input(make_nested_container(type, 65)); + AMFDecoder decoder(input, 0); + try { + decoder.load(); + } catch (const runtime_error &ex) { + return string(ex.what()) == "Maximum AMF nesting depth exceeded"; + } + return false; +} + +int main() { + const ContainerType types[] = { + ContainerType::Object, + ContainerType::EcmaArray, + ContainerType::StrictArray, + }; + + for (auto type : types) { + if (!accepts_nesting_limit(type)) { + cerr << "AMF " << container_name(type) << " at the nesting limit was not decoded" << endl; + return 1; + } + if (!rejects_excessive_nesting(type)) { + cerr << "AMF " << container_name(type) << " beyond the nesting limit was not rejected" << endl; + return 2; + } + } + + return 0; +}