Am Mon, 11 Jun 2012 23:27:45 +0200 schrieb Max Kellermann <m...@duempel.org>:
> On (broken?) MP3 files, mpg123_getformat() hangs in an I/O loop that
> reads one byte at a time, seeks back 64 kB, and repeats practically
> forever. Example strace:
Does plain mpg123 play the file?
shell$ mpg123 /path/to/file.mp3
I don't see funny things in MPD's mpg123 usage. It does
handle = mpg123_new(NULL, &error);
error = mpg123_open(handle, path_dup);
error = mpg123_getformat(handle, &rate, &channels, &encoding);
... and according to your report, it hangs there, trying to find a valid
header. The same should happen to mpg123.
> [...]
> read(4, "\277", 1) = 1
> read(4, "Y", 1) = 1
> read(4, "\36", 1) = 1
> read(4, "\v", 1) = 1
> lseek(4, -65536, SEEK_CUR) = 19013
> read(4, "\277", 1) = 1
> read(4, "Y", 1) = 1
> read(4, "\36", 1) = 1
> read(4, "\v", 1) = 1
> read(4, "\"", 1) = 1
> read(4, "`", 1) = 1
> [...]
>
Judging from that ... this must be guess_freeformat_framesize(). This is called
when a header indicating a free-format stream is encountered. That needs a
search for a following header to determine frame size. 64K is perhaps a bit
excessive here; I'd have to check that the actual maximally possible frame size
is (with low sample rate and high bit rate, you can achieve rather larger
individial frames).
> This causes the Music Player Daemon (when built with libmpg123) to go
> in an endless busy loop upon starting playback, and becomes
> irresponsive as soon as a client ask MPD to change playback. Severity
> "important" (or more) because this bug is a remote DoS vulnerability
> for MPD.
Theoretically, if the free format header search is triggered repeatedly,
eventually, mpd should finish decoding the file.
> lseek(4, -65536, SEEK_CUR) = 19013
That position should increase over time, at least ...
I see that this free format search is somewhat of a loophole. Normally, mpg123
will stop trying after encountering 64K of junk (this limit is configurable).
In case of free format headers, the search for the following one is not counted
as junk, as there could be perfectly valid headers in between that don't
qualify as following.
So, if the search fails (and we need a seek back of 64K), just the inital free
format header is discarded and counts towards the 64K of junk. We could include
some safeguards here, perhaps enforcing that this search for free format frame
size only happens once, making the regime more strict for those streams than
for normal ones (which might be reasonable as free format streams are rare).
> Due to copyright issues, I will provide a sample file demonstrating
> the problem via private email only.
I'd like to test that file myself (as mpg123 upstream, as you might have
guessed). It seems to be a rather nasty example of kicking off the free format
search repeatedly.
Alrighty then,
Thomas
signature.asc
Description: PGP signature
