Skip to content

reject zero-datasize flv tag in FlvReader::Read#3366

Open
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:flv-tag-zero-datasize
Open

reject zero-datasize flv tag in FlvReader::Read#3366
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:flv-tag-zero-datasize

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

While reading FLV tags off a playback buffer I traced a stream that came back as one oversized message with the rest of the buffer swallowed. The DataSize in a video/audio tag header is a 3-byte field read straight off the wire in FlvReader::Read; the code then consumes the single VideoTagHeader/AudioTagHeader byte and cuts msg_size - 1 body bytes. When DataSize is 0 that subtraction wraps to 0xFFFFFFFF (msg_size is uint32_t), so cutn() drains everything left in the buffer into one message and desyncs every tag after it. A malicious or corrupt HTTP-FLV source can trigger it with a single 15-byte tag.

Reject DataSize of 0 with EINVAL before any bytes are consumed, since a valid audio/video tag always carries at least the one-byte codec header that the reader immediately reads. The guard sits next to the msg_size read so the underflow can never reach cutn(), and the same fix is applied to both the video and audio readers. Added a regression test that feeds a zero-DataSize tag and checks the reader errors without draining the buffer.

@wwbmmm

wwbmmm commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

@sahvx655-wq please resolve merge conflicts

@sahvx655-wq sahvx655-wq force-pushed the flv-tag-zero-datasize branch from 3387c99 to 5020c1f Compare July 5, 2026 08:30
@sahvx655-wq

Copy link
Copy Markdown
Contributor Author

Done, rebased onto master and pushed. The conflict was only in brpc_rtmp_unittest.cpp: the AVC SPS out-of-bounds fix (#3371) landed a new test in the same spot mine sits, so I kept both tests side by side and gave each its own closing brace. rtmp.cpp merged cleanly with no changes to the guard.

Both flv_reader_rejects_zero_datasize_video_tag and _audio_tag are intact alongside avc_seq_header_sps_without_zero_byte. Should be clean to merge once CI goes green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants