Bug 307091 – media-video/mplayer-1.0_rc4_p20100213: memory corruption while loading .pls playlist

• Product: Gentoo Linux
• Status: RESOLVED UPSTREAM

[Description Michał Mirosław 2010-02-27 17:19:26 UTC]

recent mplayer (1.0_rc4_p20100213) crashes on some (maybe invalid) .pls file (example attached). Reason: memory corruption because of off-by-one bugs.


Reproducible: Always

Steps to Reproduce:
mplayer -playlist http://www.miastomuzyki.pl/n/rmfrock.pls
(copy attached)
Actual Results:  
crash:

[...]
Trying Winamp playlist...
Detected Winamp playlist format
*** glibc detected *** mplayer: realloc(): invalid old size: 0x0933aab8 ***


Expected Results:  
playing radio stream, of course ;)

[Comment 1 Michał Mirosław 2010-02-27 17:20:16 UTC]

Created attachment 221451 [details]
Playlist generating the failure

[Comment 2 Michał Mirosław 2010-02-27 17:21:22 UTC]

Created attachment 221453 [details, diff]
Quick patch to fix this issue

[Comment 3 Michał Mirosław 2010-02-27 17:26:54 UTC]

Note: The patch prevents crash for invalid entries, but might not be a proper fix.

[Comment 4 Sebastian Pipping 2010-02-27 23:52:46 UTC]

Thanks for reporting, especially nice to have a trigger file.


> Note: The patch prevents crash for invalid entries, but might not be a proper
> fix.

From a quick look at
- the playlist
- the patch
- another PLS example

It seems to me that all these "length" entries should be "Length1", "Length2" and so forth and that the lack of that number is the problem.  Ignore me if you already knew that :-)

[Comment 5 Michał Mirosław 2010-02-28 01:34:36 UTC]

There's another bug in this code anyway, as any valid file/length/whatever with some large number will cause mplayer to alloc a lot of memory or maybe even overflow the size calculations and then still corrupt memory.

[Comment 6 Michał Mirosław 2010-02-28 03:07:20 UTC]

Created attachment 221505 [details, diff]
Bigger patch to guard against malformed .pls playlists

This patch modifies .pls parser to guard against malformed .pls entries (big/no numbers). Performance difference should be negligible for small playlists, and small for big ones but with ordered and consecutive-numbered entries.

(This patch includes a simple optimization to avoid one copy per string value parsed. I can split this out if really needed.)

Compile and run tested against attached playlist.

[Comment 7 Samuli Suominen (RETIRED) 2010-04-30 13:57:41 UTC]

Was this reported upstream? Obviously it's not getting in Portage before that.

http://bugzilla.mplayerhq.hu/

[Comment 8 Michał Mirosław 2010-04-30 16:09:05 UTC]

I assumed that package maintainer would pass it on.

The patch does not apply to latest SVN snapshot - there were some related changes in upstream playtreeparser.c so this bug might actually be fixed there already (I haven't tested it yet, though).

[Comment 9 Samuli Suominen (RETIRED) 2010-04-30 20:56:24 UTC]

(In reply to comment #8)
> I assumed that package maintainer would pass it on.

In perfect world, sure. Get involved with gentoo as media-video maintainer, or get involved with mplayer upstream yourself.

> The patch does not apply to latest SVN snapshot - there were some related
> changes in upstream playtreeparser.c so this bug might actually be fixed there
> already (I haven't tested it yet, though).

Let us know,  thanks!

[Comment 10 Michał Mirosław 2010-05-01 00:53:45 UTC]

(In reply to comment #9)
> (In reply to comment #8)
> > The patch does not apply to latest SVN snapshot - there were some related
> > changes in upstream playtreeparser.c so this bug might actually be fixed there
> > already (I haven't tested it yet, though).
> Let us know,  thanks!

SVN snapshot from 2010-04-30 doesn't crash on this playlist anymore.