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 ;)
Created attachment 221451 [details] Playlist generating the failure
Created attachment 221453 [details, diff] Quick patch to fix this issue
Note: The patch prevents crash for invalid entries, but might not be a proper fix.
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 :-)
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.
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.
Was this reported upstream? Obviously it's not getting in Portage before that. http://bugzilla.mplayerhq.hu/
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).
(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!
(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.
