Reads out-of-bound in asf_mmst_streaming_start and http_build_request

[ggrieco]

Summary of the bug:

Some reads out-of-bound in functios asf_mmst_streaming_start and http_build_request are present in Mplayer 1.1-4.8 (tested in Ubuntu 14.04). Other versions are probably affected.

How to reproduce:

First, launch a dummy server:

$ true | netcat -l 127.0.0.1 5002

Then, mplayer using valgrind:

$ valgrind mplayer mms://127.0.0.1:5002
==31830== Memcheck, a memory error detector
==31830== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==31830== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==31830== Command: mplayer mms://127.0.0.1:5002
==31830==
MPlayer 1.1-4.8 (C) 2000-2012 MPlayer Team
mplayer: could not connect to socket
mplayer: No such file or directory
Failed to open LIRC support. You will not be able to use your remote control.

Playing mms://127.0.0.1:5002.
STREAM_ASF, URL: mms://127.0.0.1:5002
Resolving 127.0.0.1 for AF_INET6...

Couldn't resolve name for AF_INET6: 127.0.0.1
Connecting to server 127.0.0.1[127.0.0.1]: 5002...

Connected
==31830== Invalid read of size 4
==31830== at 0x5A6792: asf_mmst_streaming_start (asf_mmst_streaming.c:595)
==31830== by 0x5A8AA8: open_s (asf_streaming.c:94)
==31830== by 0x54FD1F: open_stream_full (stream.c:186)
==31830== by 0x54F3D0: open_stream (open.c:65)
==31830== by 0x4321D9: main (mplayer.c:3223)
==31830== Address 0x153e0ef0 is 0 bytes inside a block of size 1 alloc'd
==31830== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31830== by 0x5A65E7: asf_mmst_streaming_start (asf_mmst_streaming.c:539)
==31830== by 0x5A8AA8: open_s (asf_streaming.c:94)
==31830== by 0x54FD1F: open_stream_full (stream.c:186)
==31830== by 0x54F3D0: open_stream (open.c:65)
==31830== by 0x4321D9: main (mplayer.c:3223)
==31830==
==31830== Invalid read of size 4
==31830== at 0x5A67E6: asf_mmst_streaming_start (asf_mmst_streaming.c:597)
==31830== by 0x5A8AA8: open_s (asf_streaming.c:94)
==31830== by 0x54FD1F: open_stream_full (stream.c:186)
==31830== by 0x54F3D0: open_stream (open.c:65)
==31830== by 0x4321D9: main (mplayer.c:3223)
==31830== Address 0x153e0ef0 is 0 bytes inside a block of size 1 alloc'd
==31830== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31830== by 0x5A65E7: asf_mmst_streaming_start (asf_mmst_streaming.c:539)
==31830== by 0x5A8AA8: open_s (asf_streaming.c:94)
==31830== by 0x54FD1F: open_stream_full (stream.c:186)
==31830== by 0x54F3D0: open_stream (open.c:65)
==31830== by 0x4321D9: main (mplayer.c:3223)
==31830==

Alert! EOF
read error:: Operation now in progress
pre-header read failed
Resolving 127.0.0.1 for AF_INET6...

Couldn't resolve name for AF_INET6: 127.0.0.1
Connecting to server 127.0.0.1[127.0.0.1]: 5002...

connect error: Connection refused
Failed, exiting.
==31830== Invalid read of size 4
==31830== at 0x5AA4BA: http_build_request (http.c:478)
==31830== by 0x5AB409: http_send_request (network.c:261)
==31830== by 0x5AA827: http_streaming_start (http.c:725)
==31830== by 0x5AAF5B: open_s2 (http.c:936)
==31830== by 0x54FD1F: open_stream_full (stream.c:186)
==31830== by 0x54F3D0: open_stream (open.c:65)
==31830== by 0x4321D9: main (mplayer.c:3223)
==31830== Address 0x153ecf90 is 0 bytes inside a block of size 2 alloc'd
==31830== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31830== by 0x5AA492: http_build_request (http.c:468)
==31830== by 0x5AB409: http_send_request (network.c:261)
==31830== by 0x5AA827: http_streaming_start (http.c:725)
==31830== by 0x5AAF5B: open_s2 (http.c:936)
==31830== by 0x54FD1F: open_stream_full (stream.c:186)
==31830== by 0x54F3D0: open_stream (open.c:65)
==31830== by 0x4321D9: main (mplayer.c:3223)
==31830==
Resolving 127.0.0.1 for AF_INET6...

Couldn't resolve name for AF_INET6: 127.0.0.1
Connecting to server 127.0.0.1[127.0.0.1]: 5002...

connect error: Connection refused
No stream found to handle url mms://127.0.0.1:5002

Exiting... (End of file)

[rxt]

Confirmed with svn HEAD
The url has no path, but MPlayer is trying anyway to convert the path string to utf16.

Should be easy to fix, but I was unable to find a working mms stream to test it; I want to be sure that the fix does not break anything.

Do you have a public working mms url (better without path) to test this?

[ggrieco]

Replying to rxt:

Confirmed with svn HEAD
The url has no path, but MPlayer is trying anyway to convert the path string to utf16.

Should be easy to fix, but I was unable to find a working mms stream to test it; I want to be sure that the fix does not break anything.

Great, thanks for looking at this issue.

Do you have a public working mms url (better without path) to test this?

Before reporting this issue, i tried a few from here:

​http://data.webbie.org.uk/radioStationsHTML.php

Btw, i asked for a CVE so this bug can be patched quickly in several packages from different OS but the CVE assignement team needs more information from upstream before doing it. If you a few minutes, take a look at this post:

​http://www.openwall.com/lists/oss-security/2015/11/17/4

Thanks!

[rxt]

I made further checks on this, and I have to correct my first analysis.
The utf16 conversion code does not overread the path string; the warning in valgrind is triggered by strlen(path).

The warning can be reporduced also with a path, as long as it's less than 3 characters.
The warning disappears if the file asf_mmst_streaming.c is compiled with -O1 instead of -O2 (default in MPlayer)

From what I got by checking the copiled files the strlen() function is often inlined, but the code is different in the -O1 and -O2 case.
For -O1 the implementation uses a traditional repnz scas es:(%rdi),%al, that reads one single byte in each iteration.
For -O2 the code uses some complex algoritm with byte masks, that works on 4 bytes; this seems to be the reason of the invalid 4 byte read. From what I got it's a real overread, not a false trigger. The overread should be harmless, it can not lead to information disclosure.

There is not much that I can fix on MPlayer side; I could make a woraround by changing the malloc() call on line 537 from unescpath=malloc(strlen(path)+1); to unescpath=malloc(strlen(path)+4);, but I don't like it too much.

I'm using gcc (Debian 4.9.2-10) 4.9.2 on Debian stable 8.2.

Can you confirm my analysis?

[ggrieco]

If you are asking me, i'm not so familiar with the mplayer source code or the string function internals..

[reimar]

I think that is just the optimized glibc strlen() implementation that can overread the buffer as it "knows" it is safe.
Newer valgrind versions are supposed to filter that out with the help of debug info I believe.
Any objections to closing it? I don't think there is a real issue here, and I at least cannot reproduce it.

[ggrieco]

Sure, no problem.