Re: April report

[Brian May <bam@debian.org>]

Antoine Beaupré <anarcat@orangeseeds.org> writes:

> On 2017-04-19 19:05:36, Brian May wrote:
>
> [...]
>
>> As I have run out of hours this month, if anybody else wants to take
>> over either of these, please let me know and I will provide more
>> details.
>
> I'd take a look at the XBMC thing...

The webserver is in xbmc/network/WebServer.cpp, see AnswerToConnection()

Case (a), URL prefixed with "/vfs", we return the result of
CreateFileDownloadResponse().  The first 5 characters are removed - so
if "/vfs/" prefixed it strips the entire prefix - but curously doesn't
check the 5th character. So /vfss would also get stripped. (what happens
if we pass only 4 characters???  "/vfs"?)

Case (b), any other static file (not ending with /), we fall down to
CreateFileDownloadResponse() also. In this case I believe the first
character of the URL is always '/'.

I don't see any sanity checking of the URL, at any stage.


CreateFileDownloadResponse is in the same file. It opens a CFile to the
URL, reads it, and sends it.


CFile stuff is in xbmc/filesystem/File.cpp. CFile in turn passes
everything to CFileFactory:

CFileFactory::CreateLoader(url), which is in
xbmc/filesystem/FileFactory.cpp


The URL is parsed with CURL. xbmc/URL.cpp

For case (a) protocol used in the exploit is "special" - this comes from
the untrusted user supplied URL - now stripped of /vfs/ - and can be
anything.  As such we use CFileSpecialProtocol. (or any other possible
protocol). For case (b) (starting with '/') protocol is empty and uses
CFileHD (not to be confused with CFile which we already used).

Case (a) CFileSpecialProtocol just reads the file, and same with case
(b) CFileHD just reads the file.

./xbmc/filesystem/FileSpecialProtocol.cpp
./xbmc/filesystem/FileHD.cpp


Somewhere for case (a) there must be decoding of the special characters
in the URL. I am not sure where this decoding takes place. In my scan of
the source code I think I missed it. Might need to attach a debugger and
double check what I have said, plus see where the decoding happens.

I am speculating CURL might be the best place to strip "../" sequences
from the file name, however this really depends on where the above
decoding takes place.

Hope the above helps :-)
-- 
Brian May <bam@debian.org>