diff --git a/src/zm_stream_socket_protocol.cpp b/src/zm_stream_socket_protocol.cpp index acbe2e2f3..98fd2b09f 100644 --- a/src/zm_stream_socket_protocol.cpp +++ b/src/zm_stream_socket_protocol.cpp @@ -18,6 +18,7 @@ #include "zm_stream_socket_protocol.h" #include "zm_ffmpeg.h" +#include "zm_logger.h" #include @@ -81,7 +82,8 @@ void append_tlv_u64(std::vector &out, uint8_t tag, uint64_t value) { void append_tlv_str(std::vector &out, uint8_t tag, const std::string &value) { // TLV length is u16; clamp pathologically long strings rather than overflow. - uint16_t len = value.size() > 0xffff ? 0xffff : static_cast(value.size()); + uint16_t len = value.size() > kMaxTlvValueSize ? kMaxTlvValueSize + : static_cast(value.size()); append_tlv(out, tag, reinterpret_cast(value.data()), len); } @@ -121,7 +123,15 @@ std::vector BuildHello(const AVCodecParameters *par, AVRational frame_r append_tlv_u32(out, kTlvCodecId, static_cast(par->codec_id)); if (par->extradata and par->extradata_size > 0) { - append_tlv(out, kTlvExtradata, par->extradata, static_cast(par->extradata_size)); + if (static_cast(par->extradata_size) > kMaxTlvValueSize) { + // A TLV value is at most 64 KiB; parameter sets are a few hundred bytes, + // so anything bigger is not something a consumer could use anyway. + // Omit it rather than send a silently truncated blob. + Warning("StreamSocket: extradata of %d bytes exceeds the HELLO TLV limit" + " of %zu bytes, omitting it", par->extradata_size, kMaxTlvValueSize); + } else { + append_tlv(out, kTlvExtradata, par->extradata, static_cast(par->extradata_size)); + } } if (par->codec_type == AVMEDIA_TYPE_VIDEO) { if (par->width > 0) append_tlv_u32(out, kTlvWidth, par->width); diff --git a/src/zm_stream_socket_protocol.h b/src/zm_stream_socket_protocol.h index 16030a641..abad4e395 100644 --- a/src/zm_stream_socket_protocol.h +++ b/src/zm_stream_socket_protocol.h @@ -56,6 +56,9 @@ constexpr size_t kHeaderSize = 24; constexpr uint32_t kHeaderLengthBytes = kHeaderSize - sizeof(uint32_t); // Sanity cap on the length field; larger values mean a corrupt or hostile peer. constexpr uint32_t kMaxMessageLength = 32 * 1024 * 1024; +// Largest value a TLV can carry (u16 length field). Longer values are +// omitted (extradata) or clamped (strings) by the builders. +constexpr size_t kMaxTlvValueSize = 0xffff; enum class MessageType : uint8_t { Hello = 0x01, diff --git a/tests/zm_stream_socket_protocol.cpp b/tests/zm_stream_socket_protocol.cpp index 36b953638..8ff0755e8 100644 --- a/tests/zm_stream_socket_protocol.cpp +++ b/tests/zm_stream_socket_protocol.cpp @@ -381,3 +381,26 @@ TEST_CASE("stream_socket::ParseEvent rejects malformed input") { REQUIRE(out.code == kEventCaptureResumed); } } + +TEST_CASE("stream_socket::BuildHello omits extradata larger than a TLV") { + codec_parameters_ptr par{avcodec_parameters_alloc()}; + par->codec_type = AVMEDIA_TYPE_VIDEO; + par->codec_id = AV_CODEC_ID_H264; + std::vector huge(kMaxTlvValueSize + 1, 0x42); + set_extradata(par.get(), huge); + + std::vector payload = BuildHello(par.get(), {0, 0}); + HelloInfo info; + REQUIRE(ParseHello(payload.data(), payload.size(), info)); + REQUIRE(info.codec_id == AV_CODEC_ID_H264); + // Rather than a silently truncated blob, the tag is left out entirely + REQUIRE(info.extradata.empty()); + + // Exactly the limit still travels intact + std::vector limit(kMaxTlvValueSize, 0x24); + av_freep(&par->extradata); + set_extradata(par.get(), limit); + payload = BuildHello(par.get(), {0, 0}); + REQUIRE(ParseHello(payload.data(), payload.size(), info)); + REQUIRE(info.extradata == limit); +}