Bugzilla Bug 133988

When running mpg123 with a HTTP URL which sends _any_ HTTP redirection, mpg123
crashes:

$ mpg123 'http://patrimonium.amberfisharts.com/download.asp?lang=de&id=20'      
High Performance MPEG 1.0/2.0/2.5 Audio Player for Layer 1, 2 and 3.
Version 0.59s-r9 (2000/Oct/27). Written and copyrights by Michael Hipp.
Uses code from various people. See 'README' for more!
THIS SOFTWARE COMES WITH ABSOLUTELY NO WARRANTY! USE AT YOUR OWN RISK!
*** glibc detected *** free(): invalid pointer: 0x0808d1e8 ***
Aborted

This server sends (among others) a "Location:
downloads/Matthias%20Sachal%20-%20Patrimonium%20Theme.mp3" header, which leads
to an overflow in a heap allocated array.

This bug is caused by a Gentoo specific mpg123 patch
(103_all_CAN-2004-0982.patch), written Jeremy Huddleston
(eradicator@gentoo.org), and introduces a possible remote exploitability, which
is why I am making this bug Gentoo Dev and Security only.

--------------------------------------------
Details:
--------------------------------------------
In httpget.c:279 (after applying 103_all_CAN-2004-0982.patch), char *purl gets
assigned some heap-allocated memory, which the patch author thought to have an
upper size limit:

/* The length of purl is upper bound by 3*strlen(url) + 1 if everything in it
is a space */
purl = (char *)malloc(sizeof(char) * (strlen(url)*3 + 1));

Without the patch, this would be malloc(1024), regardless of the URL length.

In the HTTP redirection code, the redirection target is being copied into this
memory area (httpget.c:483):

strncpy (purl, request+10, 1023);

Note that here the assumption is being made that the destination is (at least)
1023 bytes in size (and, additionally, the '\0' termination was not taken care
of in case the redirection URL is >=1023 bytes long). The program crashes when
free()ing the "request" array which comes right "above" "purl" on the heap
(httpget.c:486).

(gdb) p request
$4 = 0x80811e8 "Location:
downloads/Matthias%20Sachal%20-%20Patrimonium%20Theme.mp3\r\n"
(gdb) p purl
$5 = 0x80810a0 ""

If purl got assigned a memory area sized <1023 bytes, strncpy() will overwrite
the glibc heap data structure for "request" (which comes after the memory purl
is pointing at on the heap) as it always pads the destination with zeroes.
Additionally, an attacker MIGHT be able to send a well-crafted redirection
header which overwrites this structure in a more sophisticated way, in effect
enabling him to execute arbitrary code.

--------------------------------------------
Fix:
--------------------------------------------
As a quick fix,
* purl should be malloc'd to max(1024, strlen(url)*3 + 1),
* this initial size should be recorded somewhere (similar to "request"'s
"linelength") and be used in the strncpy() call instead of the hard-coded 1023.
* zero termination: purl[purl_length-1] = 0

Beyond this heap overflow issue, redirection URLs with arbitrary length should
be taken care of.

--------------------------------------------
Notes:
--------------------------------------------
* This fix does NOT make mpg123 play the example URL above, because the
redirection header is malformed (does not supply an absoluteURI as specified in
the HTTP RFC) and mpg123 thinks "downloads" is the new host to connect to.
* This bug might be related to bug #78228.
* I'm using a recent stable x86 Gentoo, in particular sys-libs/glibc-2.3.6-r3.

Created an attachment (id=87220) [details]
103_all_CAN-2004-0982-NEW.patch

replacement for 103_all_CAN-2004-0982.patch

Created an attachment (id=87221) [details]
separate patch on top of 103_all_CAN-2004-0982.patch

this is what I changed on top of 103_all_CAN-2004-0982.patch

*** Bug 133987 has been marked as a duplicate of this bug. ***

eradicator please have a look ...

An easy way to reproduce the overflow:

( echo -ne "HTTP/1.1 302 Found\r\nLocation: "
  echo -ne "http://fooooooooooooooooooooooooooooooooooooooooooooooooo/\r\n\r\n"
)\
| nc -lp 8080 &

mpg123 http://localhost:8080/

My report is 16 days old now; eradicator does not seem to be interested at all.
Would anyone else mind to review this report? The problem description is quite
detailed, my patch is trivial, it only needs to be committed.

Gentoo should not be the only distribution out there with (another) possible
mpg123 vulnerability. (This bug was _introduced_ by a Gentoo patch.)

Sorry, I missed this when I was originally CC'd.  I'll get to it today.

Ok, all ready to commit... just waiting for the patch tarball to hit mirrors.

Thanks :) No offense, btw. (comment #6). I just hadn't heard anything from
you...

No worries.  The tarball is on mirrors, and I've marked the ebuild is marked
stable on amd64 and sparc.

Works fine for me on ~x86. Non-{amd64,sparc} stable is still vulnerable,
though.

Arches, please test 0.59s-r11 and mark stable if possible, thank you

opening

stable on ppc64

x86 done ~_~

ppc-macos done

alpha stable.

Stable on hppa

ppc stable

GLSA 200607-01

mips and ia64 don't forget to mark stable to benifit from the GLSA.

-r11 stable on mips.