Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory corruption in FLV wrapping if there is not enough allocated space for metadata #347

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lalinsky
Copy link

We had a crash when station was using long name. This is the situation:

  • You have a relay ICY stream with long stream name (e.g. >180chars, <195 chars).
  • You have a client requesting FLV wrapping.
  • The client connects to the stream before ICY metadata is sent, so the flvmeta allocation happens in flv_write_metadata where it's limited to 200 bytes.
  • It manages to add the "name" field to the flvmeta object and then maybe some small bool fields.
  • For anything larger there isn't enough space, but each time there isn't enough space, it appends "\x00\x00\x09" to the meta buffer without checking the size.
  • When this happens enough times, it overwrites the allocated space.

In the log file, it looks like this:

[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements
[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements
[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements
[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements
[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements
[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements
[2021-02-17  08:37:58] DBUG flv/flv_meta_increase 4 array elements

Then there is some other activity and then it crashes, because of the corrupted memory.

The fix does two things:

  • Makes sure that "\x00\x00\x09" is appended to the flvmeta only once, and only if there is enough space.
  • Allocates 4000 bytes in flv_write_metadata, just like it's done in mp3_set_title.

With this patch (and assuming the stream name is really super long, but below the 4000 - 5 bytes), the error log looks like this:

[2021-02-17  08:48:41] WARN flv/flv_meta_append_number not enough space for audiodatarate
[2021-02-17  08:48:41] WARN flv/flv_meta_append_number not enough space for audiosamplerate
[2021-02-17  08:48:41] WARN flv/flv_meta_append_bool not enough space for canSeekToEnd
[2021-02-17  08:48:41] WARN flv/flv_meta_append_bool not enough space for hasMetadata
[2021-02-17  08:48:41] WARN flv/flv_meta_append_bool not enough space for hasVideo
[2021-02-17  08:48:41] WARN flv/flv_meta_append_bool not enough space for hasAudio

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.

1 participant