Antoine Beaupré <[email protected]> writes: >> 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"?) > > Hmm... I am not sure, but it seems to me it takes the "right" part of > the string, starting from the "fifth" character, so whatever comes after > the "fourth" character, ie. after "/vfs". in my tests: > > http://localhost:8080/vfs/etc/passwd > > returns /etc/passwd... So just clean passthrough.
I don't think this can be correct. a URL looks like http://192.168.122.47/vfs/special://masterprofile/Thumbnails/Video/f/auto-f4b8e6fd.tbn Which on my system maps to: /root/.xbmc/userdata/Thumbnails/Video/f/auto-f4b8e6fd.tbn My believe is that the following code: if (strURL.Left(4).Equals("/vfs")) { strURL = strURL.Right(strURL.length() - 5); return CreateFileDownloadResponse(connection, strURL, methodType); } Will set strURL to: "special://masterprofile/Thumbnails/Video/f/auto-f4b8e6fd.tbn" As this starts with the format of protocol://file, CURL will decode that as protocol=="special" and file=="masterprofile/Thumbnails/Video/f/auto-f4b8e6fd.tbn". Which will then parse it off to the SpecialProtocol handler. (it is probably fine, but it always makes me nervous when I see the protocol being passed like this - I guess it means every protocol could have vulnerabilities that should be checked. Not sure if all protocols should be accessible from the website either.) As a result it may not be possible to pass an absolute path here. Not checked the case of "special:///file" however. I am guessing this CSpecialProtocol::TranslatePath function - which I can't find right now - translates the root dir to "/root/.xbmc/userdata/" on my system. Just noticed these startup messages: 07:21:43 T:140043670366272 NOTICE: special://xbmc/ is mapped to: /usr/share/xbmc 07:21:43 T:140043670366272 NOTICE: special://xbmcbin/ is mapped to: /usr/lib/xbmc 07:21:43 T:140043670366272 NOTICE: special://masterprofile/ is mapped to: /home/brian/.xbmc/userdata 07:21:43 T:140043670366272 NOTICE: special://home/ is mapped to: /home/brian/.xbmc 07:21:43 T:140043670366272 NOTICE: special://temp/ is mapped to: /home/brian/.xbmc/temp >> 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 '/'. > > Yeah, that's pretty much what I see as well, but I'm not sure how it > works out in the end - those all give 404s: > > http://localhost:8080/temp/xbmc.log > http://localhost:8080/.xbmc/temp/xbmc.log > http://localhost:8080/etc/passwd > > so i'm not sure that's such a good vector. I suspect these paths somehow map to /usr/share/xbmc/addons/webinterface.default/ - not sure how this is done however. For loading the *.css and *.js files for the website. I tried to bring up xbmc to test this theory out, however it is not obliging right now. > I just love reading the XBMC source code... Next is a URL factory? :p Yes, agreed. >> The URL is parsed with CURL. xbmc/URL.cpp > > And here I was assuming this was cURL. silly me. would have given even > amusing consequences though. Same here, it confused me at first too. > This is one nasty horrible bug, if you ask me. Yes, agreed. I suspect it is going to require somebody who has a good understanding of the code to fix it properly (as opposed to just fixing some use cases). I am now inclined the think that the correct fix might be the backend; that is every backend. Maybe turn every filename into an absolute path and check that it has the correct prefix (might be treaky for some backends). If the checks fail have Open return False - looks like how errors are handled (this code is self documented!). Not sure if this might break other things. I haven't checked recently if upstream have a fix (I doubt it) however I am doubtful that they are going to be interested in fixing the old version in Jessie or Wheezy. -- Brian May <[email protected]>
