13 ms·

If your code accepts URIs as input, filter out “file://”

370 points by stevekemp 10y ago

fpgeek 10y ago
The original link seems down, but it has been saved at archive.org: http://web.archive.org/web/20160912105232/https://blog.steve.fi/If_your_code_accepts_URIs_as_input__.html http://web.archive.org/web/20160912105232/https://blog.steve...
- stevekemp 10y ago
  Looks fine for me (the author), but glad to see a cached copy the content is pretty minimal. Rate-limiting hasn't kicked in, and I'm seeing a steady stream of visitors.
  - tomhoward 10y ago
    It seems to be inaccessible for me here in Australia, but works when I access it via a VPN to Europe. Possibly traffic is blocked from certain networks/regions.
hughperkins 10y ago
Wrong way around: only allow http:// http:// and https:// https:// (and generally filtering out anything thats not letters, numbers, slash or dot is probably a good idea. Remove any sequences of more than one slash or dot.
- mootothemax 10y ago
  >Wrong way around: only allow http:// http:// and https:// https:// For myself, it's a subtle change in developer thinking - "what should I allow" vs "what should I exclude" - that's paid off massively over the years.
  - artursapek 10y ago
    https://en.m.wikipedia.org/wiki/Robustness_principle https://en.m.wikipedia.org/wiki/Robustness_principle
    - taneq 10y ago
      Didn't we decide in the aftermath of IE6 that this was a bad idea, and that we should be strict in both what we accept and what we emit?
      - gsnedders 10y ago
        No, we decided that what was important was interoperable implementations: it doesn't matter how you achieve that goal. What's needed is specs that define how to handle all input (it doesn't matter what the spec says: it can define how to handle every single last possible case as HTML5 does, or it can define a subset of inputs to trigger some fatal error handling as XML1.0 does) and sufficient test suites that implementers catch bugs in their code before it ships (and the web potentially starts relying on their quirks). The problem with IE6 was the fact that it wasn't interoperable (in many cases, every implementation was conforming according to the spec, and there were frequently differences in behaviour in valid input that the spec didn't fully define) and the fact that it had lots of proprietary extensions (and being strict and disallowing any extensions makes it hard to extend formats in general in a non-proprietary way; one option is strict versioning but then you end up with a million if statements all over the implementation to alter behaviour depending on the version of the content). Some of the worst issues with IE that took the longest for other browsers to match were things like table layout: IE quite closely matched NN4 having invested a lot in reverse-engineering that as the web depended on the NN4 behaviour in places; Gecko had rewritten all the table layout code from NN and didn't match its behaviour having been written according to the specs which scarcely define how to layout any table even today.
    - mootothemax 10y ago
      >https://en.m.wikipedia.org/wiki/Robustness_principle https://en.m.wikipedia.org/wiki/Robustness_principle You need to be careful of where you place the emphasis on that, though: Be -liberal- in what you accept. vs Be liberal in what you -accept-.
- buro9 10y ago
  Exactly. Whitelist only trusted schemes, do not wait to blacklist untrusted. I wrote the Go HTML sanitizer: https://github.com/microcosm-cc/bluemonday https://github.com/microcosm-cc/bluemonday and have a rule for user generated (untrusted) content that basically does whitelist just the things that one can trust: https://github.com/microcosm-cc/bluemonday/blob/master/helpers.go#L124-L141 https://github.com/microcosm-cc/bluemonday/blob/master/helpe... That states that URIs must be: 1. Parseable 2. Relative 3. Or one of: mailto http https 4. And that I will add rel="nofollow" to external links, and additionally I'll add "rel="noopener" if the link has a target="_blank" attribute Oh, and I do not trust Data URIs either.
  - deleted 10y ago
    [deleted]
  - stirner 10y ago
    Might want to add tel to the whitelist. It works in roughly the same way as mailto but interfaces with telephone apps instead of email clients.
    - buro9 10y ago
      This is the default user-generated policy, others are able to tweak and adjust using policy rules, i.e: p.AllowURLSchemes("tel") I chose conservative and safe defaults, not everyone wishes to whitelist telephone links.
- willvarfar 10y ago
  (Also be weary of imagetragick-type bugs too, where the URL starts innocuously and then contains some shellcode, because you pass the URL to something that'll paste it into system() call)
  - throwanem 10y ago
    I certainly am weary of bug branding...
    - willvarfar 10y ago
      I know, me too, but I didn't name it. There's a section on the main page imagetragick.com talking about branding and how they got no traction without one.
    - nerdponx 10y ago
      I only started seeing this weary/wary misspelling in recent years. They don't sound alike, and they don't really look alike. Did cell phone spellcheckers give rise to this one?
      - willvarfar 10y ago
        'weary' means tired of something, 'wary' means cautious of something.
        nerdponx 10y ago
        Right, and in the last year or so I've started to see people getting these terms confused.
        barefootcoder 10y ago
        Because our language is a mashup of multiple other languages, and thus the rules are inconsistent. As for pronunciation of those two: weary is pronounced like "ear" wary is pronounced like "air"
      - 10y ago
- pdkl95 10y ago
  Can we please stop trying to enumerate badness[1]? When parsing input it is possible to define the set of valid input, not all possible invalid inputs. Also, anybody accepting input from an untrusted source (such as anything from a network or the user) that isn't verifying the data with a formal recognizer is doing it wrong[2]. Instead of writing another weird machine, guarantee that the input is valid with a parser generator (or whatever) recognize the input and drop anything even slightly invalid. [1] http://www.ranum.com/security/computer_security/editorials/dumb/ http://www.ranum.com/security/computer_security/editorials/d... [2] https://media.ccc.de/v/28c3-4763-en-the_science_of_insecurity https://media.ccc.de/v/28c3-4763-en-the_science_of_insecurit...
  - ptero 10y ago
    I agree with the approach. However, specific examples of different badnesses are useful for testing the final product.
    - kazinator 10y ago
      I.e. move enumeration of badness from code into test-cases.
  - ubernostrum 10y ago
    guarantee that the input is valid with a parser generator OK, that works really well... until you learn how much non-RFC-specified behavior is built in to web browsers. Simply building a parser to the RFC will leave you wide open to all sorts of nastiness! The is_safe_url() internal function in Django is a bit of a historical dive into things we've learned about how browsers interpret (or, arguably, misinterpret) various types of oddball URLs: https://github.com/django/django/blob/master/django/utils/http.py#L287 https://github.com/django/django/blob/master/django/utils/ht...
    - oneplane 10y ago
      I do wonder; is there any browser that is actually full-RFC-specced? I checked a few (the mainstream desktop ones, but also links2 etc.), but so far they all seem to have glue to fix historical behavior.
- robert_tweed 10y ago
  Just as a concrete example of why this is the right approach, there is at least one enterprise CMS that installs a custom protocol handler that can be use to access any object stored in the CMS if you know or can guess/discover its URI. It's worth assuming there are others that you don't know about. The other advantage of the whitelist approach here is that you know exactly which protocols you think you support and can design tests for them. For instance to support https, you'll want to check you have decent error handling and do not silently accept potential MitM certificates.
- avian 10y ago
  > filtering out anything thats not letters, numbers, slash or dot is probably a good idea. This is highly non-trivial once you realize that the world speaks more than ASCII and things like http://www.xn--n3h.net http://www.xn--n3h.net exist.
  - mootothemax 10y ago
    >This is highly non-trivial once you realize that the world speaks more than ASCII and things like http://www.xn--n3h.net http://www.xn--n3h.net exist. I was under the impression that requests to and from the server still used ASCII? That is, the server would see a host header as this: Host: www.xn--n3h.net And not as this: Host: www.[snowman icon].net Anything else is a question of URL-encoding, which if not used would raise interesting bugs with space characters, let alone anything more exotic like snowmen. Edit for completeness: in my server logs, the GET request for a /[snowman icon] URL is url encoded to GET /%E2%98%83 HTTP/1.1
    - tokenizerrr 10y ago
      Right but how does the user submit it and what do you put in the href?
      - kijin 10y ago
        If a user copied the URL from the address bar, it will be correctly percent-encoded already. You can put the same percent-encoded URL in the href attribute of a hyperlink. A properly encoded URL will not contain any character that requires escaping in an HTML context. When a user clicks on that link, the browser will navigate to the percent-encoded URL but display the snowman icon in the address bar. If the user copies it, it will transparently turn back into the percent-encoded URL. All modern browsers do this.
        germanier 10y ago
        I just tried doing that with a few domain names containing an umlaut (äöü) and every single time that letter was copied into the clipboard (even though behind the scenes at the request level it would have been encoded). This is what I expect as a regular user. They don't want to deal with encoded, unreadable URLs.
- wodenokoto 10y ago
  I believe uri today accept all sorts of characters outside ascii.
- busterarm 10y ago
  But if your code returns a URL, please don't do this. You should allow PRURLs (protocol-relative URLs). As in "//url". Especially you, Hubspot. I should be able to set a protocol-relative thank you page URL on your forms. If my user reaches your embedded form on my page as http, you should give them http. If they do it on https, it should give them https. Yes, I have an axe to grind.
- SnacksOnAPlane 10y ago
  You seriously want to disallow gopher://? C'mon, man!
  - deleted 10y ago
    [deleted]
- zwily 10y ago
  Also make sure to fully resolve the DNS down to all possible IP addresses, and verify that they are all external to your network. And if you're on EC2, make sure nobody is hitting 169.254.169.254. Really, there are so many gotchas around fetching user-supplied URLs that it's scary.
  - yuliyp 10y ago
    And be sure to check it again if there's a redirect. Don't let your URL handling library do this. Alternately send all your traffic through a proxy that can't talk into your network.
  - l_zzie 10y ago
    Importantly, fetching DNS twice (once to check, another to download) is an incomplete solution, since DNS responses can change (cf "DNS rebinding").
  - captn3m0 10y ago
    I remember this was exactly how a readability service (readability or instapaper or something similar, can't recall now) was attacked. The service allowed you to fetch internal urls and presented them formatted on your phone. A mixture of file:// and internal web urls allowed complete takeover.
    - alxndr 10y ago
      Any chance you could dig up the details on that?
- 1propionyl 10y ago
  This is the correct answer and best practice. Be conservative in what you accept and liberal in what you produce.
- zero_iq 10y ago
  Also, fully decode the input string before doing this processing to make sure you really find all those sequences. A simple, seemingly obvious step that a surprising amount of software neglects to do.
milankragujevic 10y ago
I usually only do this, if I'm putting a user-submitted URL into file_get_contents: if(substr($_GET['url'], 0, 4) != 'http') { exit; }
- mootothemax 10y ago
  > file_get_contents FYI, if you ever need a quick performance boost, switch to using curl for these calls. The difference is noticeable to the naked eye.
- philo23 10y ago
  If thats the only validation on calls to file_get_contents, that could very easily be bypassed. Entering something like just "/etc/passwd" for example.
  - milankragujevic 10y ago
    /etc != http :)
    - philo23 10y ago
      Ah yes, you're right. Totally mis-read the code. It'd still leave access to any files in the same (or sub) directory starting with http, which realistically would probably be none but still something to bear in mind.
      - nitrogen 10y ago
        Right, best to check the whole URL. Something like "http/../../../etc/passwd" might get through otherwise.
        milankragujevic 10y ago
        Like.... $url = rawurldecode($_GET['url']); $url_without_protocol = str_replace(array('https://', 'http://'), '', $url); $protocol = (stristr($url, 'https://') ? 'https' : 'http'); $page = file_get_contents($protocol . '://' . $url_without_protocol);
greenleafjacob 10y ago
This is the confused deputy problem. The most general solution to this class of vulnerabilities, SELinux, has been largely ignored. Does SELinux need more work to "bring it to market", or is it just too complicated and needs to be simplified?
- pawadu 10y ago
  configuring SELinux is way too complicated for the average user.
  - nerdponx 10y ago
    And poorly documented. I learned what little I know about it from online tutorials, not the docs
  - zanny 10y ago
    Then use one of the nowadays many alternatives - there is Apparmor, Tomoyo, and GRSec's RBAC all performing the same MAC job.
- willvarfar 10y ago
  SELinux is a real burden for even motivated sysadmins. However, if you have a single image that you are going to make millions of copies of then the effort vs reward might slant in SELinux's favour, e.g. Android does use SELinux.
- rndgermandude 10y ago
  SELinux is more the stopgap measure when everything else failed already, or at least it should be and prevent the most harmful things like reading random stuff from /etc. It is not something I'd say of "I got SELinux, now I don't need to validate user input". In the concrete example from the article, the process needs to access to /etc/hosts to do name resolutions, yet it should not send this information out to who knows who. How do you model that as a SELinux config? You cannot really. Unless you introduce dedicated (class of uncoupled, distinct, identifiable class of) processes acting as agents for resolving hosts with the help of /etc/hosts and whitelist them in SELinux... Which adds a whole lot of complexity. And you still have to make sure your new fancy agents cannot be tricked into giving up sensitive information. So at the end of the day, you should do defense in depth which of course should include user input validation and probably SELinux as well.
Grue3 10y ago
I tried Python requests and Common Lisp drakma, and neither of them can handle "file://" URL schema. Which HTTP client libraries are actually vulnerable to this?
- simscitizen 10y ago
  The most popular HTTP library is libcurl, and it supports file URLs by default.
- jwilk 10y ago
  Python's urllib(2), Perl's LWP
- mapleoin 10y ago
  urllib/urllib2's urlopen for example. I would assume it would be a bug for the requests library to allow file:///
- konceptz 10y ago
  Not to nitpick but it's not really a vulnerability. It's including the correct protocols for URIs.
Eridrus 10y ago
This is a special case of a Server-Side Request Forget vulnerability. Validating schemes is part of the answer but not the whole answer because attackers can still forge requests to internal resources you have firewalled off from the internet. These were recently released to help people deal with these issues since the details can be finicky: http://blog.includesecurity.com/2016/08/safeurl-server-side-request-forgery-protection-library.html http://blog.includesecurity.com/2016/08/safeurl-server-side-...
- RegW 10y ago
  Really. Surely if I tried to enter http://localhost:8983/solr/admin/cores?action=UNLOAD&core=core0&deleteIndex=true http://localhost:8983/solr/admin/cores?action=UNLOAD&core=co... the back service would be password protected. Oh, wait a minute.
- Eridrus 10y ago
  Should say "Server-Side Request Forgery", damn mobile.
jwilk 10y ago
> Actually the actual output all newlines had been stripped. Not stripped, but replaced by spaces. Also, the linked image looks like /etc/passwd, not /etc/hosts. > Weird. Not weird. That's how whitespace in HTML works.
- kijin 10y ago
  OP tested his hack on an online Markdown converter, so it probably has more to do with Markdown's treatment of whitespace than HTML's. As it happens, Markdown ignores most non-consecutive newlines.
  - detaro 10y ago
    It's a converter taking HTML as input, converting it to Markdown.
wkd 10y ago
"Despite reporting the problem to the author on Friday, and following up the report via Twitter this has not yet been fixed, but after four days I assume I'm not alone in spotting this." Giving someone a weekend to fix something doesn't exactly sound like responsible disclosure. I understand if you get excited because you found a flaw but if you find something like this please be more responsible with publishing your findings.
- kuschku 10y ago
  Well, I found the same by pure chance before reading this article, and I suspect many more in the HN crowd did. If already half a dozen people on HN report they’ve found it and emailed the person about it, it’s likely it’s too late for responsible disclosure.
  - 4ndr3vv 10y ago
    Agreed! Better get that karma before someone else does /snark
    - kuschku 10y ago
      The issue is more that if so many people have already found it, who else has?
      - 23andwalnut 10y ago
        Disclosing it publicly before it's been fixed only increases the number.
        pg_is_a_butt 10y ago
        not disclosing it publicly when it's likely already known, only increases the number of victims as lack of pressure doesn't get the issue fixed immediately. am i doing it right? if you want to offer always on public services, then you have to have an always on company. always.
Bino 10y ago
There are others things to consider when fetching data from untrusted HTTP resources. A. Can it be used to DDoS B. How much data do you read (download, inbound may cost you real money)
r1ch 10y ago
Also be sure not to allow loopback connections (ie your own site or localhost) or you can cause a deadlock if the user requests the same URL to download from your site recursively. Choose an appropriate timeout too to prevent users tying up backend processes with a HTTP server that is slow to respond, and don't follow Location headers to avoid bypassing of your initial filters. A Range header should also be used to prevent users from telling your server to download multi-GB files and causing bandwidth waste / denial of service.
- JoshTriplett 10y ago
  > Also be sure not to allow loopback connections (ie your own site or localhost) or you can cause a deadlock if the user requests the same URL to download from your site recursively. Keep in mind that any arbitrary domain can point DNS to a loopback or LAN address. So the code that fetches a URL needs to include this filtering. > don't follow Location headers to avoid bypassing of your initial filters Often, you'll want to follow redirects, but re-apply your filters when doing so. > A Range header should also be used to prevent users from telling your server to download multi-GB files and causing bandwidth waste / denial of service. You can't count on support for Range, and the server might also just behave unexpectedly. Your fetching code needs to limit how much data it accepts from a server, and drop the connection after some upper bound.
  - detaro 10y ago
    > Your fetching code needs to limit how much data it accepts from a server, and drop the connection after some upper bound. And if you accept compressed responses, remember that they can be a lot bigger when decompressed: e.g. gzip allows for factor 1000.
throwawayReply 10y ago
Also make sure you don't follow 301/302, or someone can set up a http link which redirects to file:// .
- kuschku 10y ago
  Or just, when following 301/302, call the same function again – which then validates the link completely again.
  - throwawayReply 10y ago
    I'm not sure why you're getting downvoted, it's a little unfair for people to downvote a sensible suggestion without explaining why. One possible downside is that someone could Redirect A -> B and redirect B -> A, which risks tying up your resources following links, but browsers limit how many redirects will be followed, so it ought to be possible to limit redirects.
- 3pt14159 10y ago
  Wait, what? Really!? Can this be used to get shell access somehow? I'm having trouble figuring out how you go from Chrome opening a remote file to _bad thing happens_.
  - throwawayReply 10y ago
    It's not necessarily browsers, they often mitigate this. The problem is with services that consume other content. For example you might have a service which generates thumbnails of sites. That service might GET https://attacker.example.org/301.html https://attacker.example.org/301.html which itself might 301 back to file:///etc/passwd . If there is insufficient validation then a screenshot of the contents of /etc/passwd might be returned by the service. All of that happens outside the context of browsers and sandboxing. For more of that kind of thing, here's an interesting write up on some vulnerabilities found in Pocket. https://www.gnu.gl/blog/Posts/multiple-vulnerabilities-in-pocket/ https://www.gnu.gl/blog/Posts/multiple-vulnerabilities-in-po...
- 10y ago
majke 10y ago
file:// urls are awesome! People forget about them and nobody actually understands what to expect from these urls. Consider cross-origin policy implications. Is file:///home/john/bar.html on the same origin as file:///home/john/foo.html ? Also, XHR is totally allowed to go to file://
beilharz 10y ago
fuckyeahmarkdown fixed it just now.
byuu 10y ago
Also be very wary of ../ or possibly ..\ in URIs. Say you have http://example.org/download?file=release/software-1.0.zip http://example.org/download?file=release/software-1.0.zip so that you can log download statistics. If you just fetch GET["download"] and return it, you're gonna have a bad time if they try http://example.org/download?file=../../../../../etc/hosts http://example.org/download?file=../../../../../etc/hosts (browsers strip it out automatically, but it's easy enough to type such a request into a telnet session.) And don't just assume your safeguard works. Act like a hacker and try it out on yourself to make sure it works. Add an assert(validate("../") == false); at startup on your server, so it won't even run otherwise. Forbid the use of fopen() and instead go through your own file::open() function that calls validate() internally, then #define fopen ERROR_DONT_USE after (or whatever equivalent for the language you use.) It is a hostile world, you can never be too safe by adding multiple (even seemingly redundant) layers of protection.
- shocks 10y ago
  Also, you can't just find and replace ../ Consider: ....// which becomes ../
- idbehold 10y ago
  I've done something similar in node to ensure URIs like those cannot walk up past a base directory: function saferesolve(base, target) { var targetPath = '.' + path.posix.normalize('/' + target) return path.posix.resolve(base, targetPath) } saferesolve("./datasource", "a/b") === "./datasource/a/b" saferesolve("./datasource", "a/b/../c") === "./datasource/a/c" saferesolve("./datasource", "../..") === "./datasource" saferesolve("./datasource", "../../a/b") === "./datasource/a/b" saferesolve("./datasource", "../../a/b/..") === "./datasource/a"
  - ajanuary 10y ago
    I've done something similar in Java, though more focused on file paths https://github.com/aJanuary/basepath https://github.com/aJanuary/basepath
konceptz 10y ago
Quick FYI. It's difficult to filter URIs with regex because they are context free grammars and belong to a larger language than regular expressions. Recursive descent parsers are the way to go like uri objects in most languages. Ex. URL url = URL.fromString(inputString); url.getScheme;
rybosome 10y ago
The vulnerability is happening because the web server is running as a user with elevated permissions. If it were running as a user who only had permissions to read files from the webserver's serving directory this would obviate the problem. Not that you shouldn't also validate input (in a whitelist rather than blacklist as others have said): just goes to show that security is a multifaceted problem with lots of ways to be paranoid. :)
- nebulous1 10y ago
  You don't need elevated privileges to read /etc/passwd
  - daveloyall 10y ago
    That is correct. sebboh@namagiri:~$ ls -al /etc/passwd -rw-r--r-- 1 root root 1450 Jun 7 09:08 /etc/passwd So, there's this idea of running publicly facing services as a user which has less privileges than a normal interactive user. This user might be called 'nobody' or 'apache', etc. However, on your average distribution, /etc/passwd is accessible to the 'nobody' user or the 'apache' user. There are various work arounds: + run your service in a chroot or jail. + employ some kind of SE Linux thing. + ...? I've only ever used the first one.
    - delinka 10y ago
      Since ages now: https://en.wikipedia.org/wiki/Shadow_password https://en.wikipedia.org/wiki/Shadow_password
      - daveloyall 10y ago
        That doesn't mean you should allow your app to access `file://` ... (Nor allow it out of a chroot.) Simply knowing some usernames on your system could provide an attacker with clues...
kazinator 10y ago
Don't write server code that opens URI's that come in as input, period. If you take URI's as input, do it only to turn them around and spit them out into some Javascript sent back to the same session. Whatever can or cannot be accessed this way is the browser's problem.
- elmigranto 10y ago
  This works as long as your business model doesn't have opening user's URIs at its core, e.g. monitoring service.
homakov 10y ago
Also, if you're using ruby's open-uri: http://sakurity.com/blog/2015/02/28/openuri.html http://sakurity.com/blog/2015/02/28/openuri.html
ara24 10y ago
This is no different than failing to validate input parameters, and letting an attacker take a dump of your database. A service connecting to a remote location should have checked the URI scheme.
ppierald 10y ago
The u= parameter in the OPs article also is vulnerable (even if http/https are whitelisted, file:// blacklisted, etc) to the #10 vulnerability on the OWASP Top Ten 2013 list, namely Unvalidated Rediredcts and Forwards. https://www.owasp.org/index.php/Top_10_2013-A10-Unvalidated_Redirects_and_Forwards https://www.owasp.org/index.php/Top_10_2013-A10-Unvalidated_... I can easily get you to click the link to drive-by malware, adult sites, pharma, phishing, etc. because the site doesn't ensure where the link is actually going to.
_RPM 10y ago
a blacklist is generally not a good idea in any case. You should use a whitelist instead.
int_19h 10y ago
I think handling (rejecting) this in the libraries that handle URIs would be the wrong fix. While this is a vulnerability in a web app, it is a handy feature in an app that runs locally, and the library doesn't really know the context. It's up to the app to filter the URLs according to its isolation requirements.
phibit 10y ago
Another fun one that's becoming more common with cloud hosted services: access to internal metadata services that issue credentials. Blocking file:// doesn't prevent access from > http://169.254.169.254/latest/meta-data/iam/security-credentials/.. http://169.254.169.254/latest/meta-data/iam/security-credent....

370 points by stevekemp 10y ago

The original link seems down, but it has been saved at archive.org: http://web.archive.org/web/20160912105232/https://blog.steve.fi/If_your_code_accepts_URIs_as_input__.html http://web.archive.org/web/20160912105232/https://blog.steve...

stevekemp 10y ago
Looks fine for me (the author), but glad to see a cached copy the content is pretty minimal. Rate-limiting hasn't kicked in, and I'm seeing a steady stream of visitors.
- tomhoward 10y ago
  It seems to be inaccessible for me here in Australia, but works when I access it via a VPN to Europe. Possibly traffic is blocked from certain networks/regions.

hughperkins 10y ago

Wrong way around: only allow http:// http:// and https:// https:// (and generally filtering out anything thats not letters, numbers, slash or dot is probably a good idea. Remove any sequences of more than one slash or dot.

mootothemax 10y ago
>Wrong way around: only allow http:// http:// and https:// https:// For myself, it's a subtle change in developer thinking - "what should I allow" vs "what should I exclude" - that's paid off massively over the years.
- artursapek 10y ago
  https://en.m.wikipedia.org/wiki/Robustness_principle https://en.m.wikipedia.org/wiki/Robustness_principle
  - taneq 10y ago
    Didn't we decide in the aftermath of IE6 that this was a bad idea, and that we should be strict in both what we accept and what we emit?
    - gsnedders 10y ago
      No, we decided that what was important was interoperable implementations: it doesn't matter how you achieve that goal. What's needed is specs that define how to handle all input (it doesn't matter what the spec says: it can define how to handle every single last possible case as HTML5 does, or it can define a subset of inputs to trigger some fatal error handling as XML1.0 does) and sufficient test suites that implementers catch bugs in their code before it ships (and the web potentially starts relying on their quirks). The problem with IE6 was the fact that it wasn't interoperable (in many cases, every implementation was conforming according to the spec, and there were frequently differences in behaviour in valid input that the spec didn't fully define) and the fact that it had lots of proprietary extensions (and being strict and disallowing any extensions makes it hard to extend formats in general in a non-proprietary way; one option is strict versioning but then you end up with a million if statements all over the implementation to alter behaviour depending on the version of the content). Some of the worst issues with IE that took the longest for other browsers to match were things like table layout: IE quite closely matched NN4 having invested a lot in reverse-engineering that as the web depended on the NN4 behaviour in places; Gecko had rewritten all the table layout code from NN and didn't match its behaviour having been written according to the specs which scarcely define how to layout any table even today.
  - mootothemax 10y ago
    >https://en.m.wikipedia.org/wiki/Robustness_principle https://en.m.wikipedia.org/wiki/Robustness_principle You need to be careful of where you place the emphasis on that, though: Be -liberal- in what you accept. vs Be liberal in what you -accept-.
buro9 10y ago
Exactly. Whitelist only trusted schemes, do not wait to blacklist untrusted. I wrote the Go HTML sanitizer: https://github.com/microcosm-cc/bluemonday https://github.com/microcosm-cc/bluemonday and have a rule for user generated (untrusted) content that basically does whitelist just the things that one can trust: https://github.com/microcosm-cc/bluemonday/blob/master/helpers.go#L124-L141 https://github.com/microcosm-cc/bluemonday/blob/master/helpe... That states that URIs must be: 1. Parseable 2. Relative 3. Or one of: mailto http https 4. And that I will add rel="nofollow" to external links, and additionally I'll add "rel="noopener" if the link has a target="_blank" attribute Oh, and I do not trust Data URIs either.
- deleted 10y ago
  [deleted]
- stirner 10y ago
  Might want to add tel to the whitelist. It works in roughly the same way as mailto but interfaces with telephone apps instead of email clients.
  - buro9 10y ago
    This is the default user-generated policy, others are able to tweak and adjust using policy rules, i.e: p.AllowURLSchemes("tel") I chose conservative and safe defaults, not everyone wishes to whitelist telephone links.
willvarfar 10y ago
(Also be weary of imagetragick-type bugs too, where the URL starts innocuously and then contains some shellcode, because you pass the URL to something that'll paste it into system() call)
- throwanem 10y ago
  I certainly am weary of bug branding...
  - willvarfar 10y ago
    I know, me too, but I didn't name it. There's a section on the main page imagetragick.com talking about branding and how they got no traction without one.
  - nerdponx 10y ago
    I only started seeing this weary/wary misspelling in recent years. They don't sound alike, and they don't really look alike. Did cell phone spellcheckers give rise to this one?
    - willvarfar 10y ago
      'weary' means tired of something, 'wary' means cautious of something.
      - nerdponx 10y ago
        Right, and in the last year or so I've started to see people getting these terms confused.
      - barefootcoder 10y ago
        Because our language is a mashup of multiple other languages, and thus the rules are inconsistent. As for pronunciation of those two: weary is pronounced like "ear" wary is pronounced like "air"
    - 10y ago
pdkl95 10y ago
Can we please stop trying to enumerate badness[1]? When parsing input it is possible to define the set of valid input, not all possible invalid inputs. Also, anybody accepting input from an untrusted source (such as anything from a network or the user) that isn't verifying the data with a formal recognizer is doing it wrong[2]. Instead of writing another weird machine, guarantee that the input is valid with a parser generator (or whatever) recognize the input and drop anything even slightly invalid. [1] http://www.ranum.com/security/computer_security/editorials/dumb/ http://www.ranum.com/security/computer_security/editorials/d... [2] https://media.ccc.de/v/28c3-4763-en-the_science_of_insecurity https://media.ccc.de/v/28c3-4763-en-the_science_of_insecurit...
- ptero 10y ago
  I agree with the approach. However, specific examples of different badnesses are useful for testing the final product.
  - kazinator 10y ago
    I.e. move enumeration of badness from code into test-cases.
- ubernostrum 10y ago
  guarantee that the input is valid with a parser generator OK, that works really well... until you learn how much non-RFC-specified behavior is built in to web browsers. Simply building a parser to the RFC will leave you wide open to all sorts of nastiness! The is_safe_url() internal function in Django is a bit of a historical dive into things we've learned about how browsers interpret (or, arguably, misinterpret) various types of oddball URLs: https://github.com/django/django/blob/master/django/utils/http.py#L287 https://github.com/django/django/blob/master/django/utils/ht...
  - oneplane 10y ago
    I do wonder; is there any browser that is actually full-RFC-specced? I checked a few (the mainstream desktop ones, but also links2 etc.), but so far they all seem to have glue to fix historical behavior.
robert_tweed 10y ago
Just as a concrete example of why this is the right approach, there is at least one enterprise CMS that installs a custom protocol handler that can be use to access any object stored in the CMS if you know or can guess/discover its URI. It's worth assuming there are others that you don't know about. The other advantage of the whitelist approach here is that you know exactly which protocols you think you support and can design tests for them. For instance to support https, you'll want to check you have decent error handling and do not silently accept potential MitM certificates.
avian 10y ago
> filtering out anything thats not letters, numbers, slash or dot is probably a good idea. This is highly non-trivial once you realize that the world speaks more than ASCII and things like http://www.xn--n3h.net http://www.xn--n3h.net exist.
- mootothemax 10y ago
  >This is highly non-trivial once you realize that the world speaks more than ASCII and things like http://www.xn--n3h.net http://www.xn--n3h.net exist. I was under the impression that requests to and from the server still used ASCII? That is, the server would see a host header as this: Host: www.xn--n3h.net And not as this: Host: www.[snowman icon].net Anything else is a question of URL-encoding, which if not used would raise interesting bugs with space characters, let alone anything more exotic like snowmen. Edit for completeness: in my server logs, the GET request for a /[snowman icon] URL is url encoded to GET /%E2%98%83 HTTP/1.1
  - tokenizerrr 10y ago
    Right but how does the user submit it and what do you put in the href?
    - kijin 10y ago
      If a user copied the URL from the address bar, it will be correctly percent-encoded already. You can put the same percent-encoded URL in the href attribute of a hyperlink. A properly encoded URL will not contain any character that requires escaping in an HTML context. When a user clicks on that link, the browser will navigate to the percent-encoded URL but display the snowman icon in the address bar. If the user copies it, it will transparently turn back into the percent-encoded URL. All modern browsers do this.
      - germanier 10y ago
        I just tried doing that with a few domain names containing an umlaut (äöü) and every single time that letter was copied into the clipboard (even though behind the scenes at the request level it would have been encoded). This is what I expect as a regular user. They don't want to deal with encoded, unreadable URLs.
wodenokoto 10y ago
I believe uri today accept all sorts of characters outside ascii.
busterarm 10y ago
But if your code returns a URL, please don't do this. You should allow PRURLs (protocol-relative URLs). As in "//url". Especially you, Hubspot. I should be able to set a protocol-relative thank you page URL on your forms. If my user reaches your embedded form on my page as http, you should give them http. If they do it on https, it should give them https. Yes, I have an axe to grind.
SnacksOnAPlane 10y ago
You seriously want to disallow gopher://? C'mon, man!
- deleted 10y ago
  [deleted]
zwily 10y ago
Also make sure to fully resolve the DNS down to all possible IP addresses, and verify that they are all external to your network. And if you're on EC2, make sure nobody is hitting 169.254.169.254. Really, there are so many gotchas around fetching user-supplied URLs that it's scary.
- yuliyp 10y ago
  And be sure to check it again if there's a redirect. Don't let your URL handling library do this. Alternately send all your traffic through a proxy that can't talk into your network.
- l_zzie 10y ago
  Importantly, fetching DNS twice (once to check, another to download) is an incomplete solution, since DNS responses can change (cf "DNS rebinding").
- captn3m0 10y ago
  I remember this was exactly how a readability service (readability or instapaper or something similar, can't recall now) was attacked. The service allowed you to fetch internal urls and presented them formatted on your phone. A mixture of file:// and internal web urls allowed complete takeover.
  - alxndr 10y ago
    Any chance you could dig up the details on that?
1propionyl 10y ago
This is the correct answer and best practice. Be conservative in what you accept and liberal in what you produce.
zero_iq 10y ago
Also, fully decode the input string before doing this processing to make sure you really find all those sequences. A simple, seemingly obvious step that a surprising amount of software neglects to do.

milankragujevic 10y ago

I usually only do this, if I'm putting a user-submitted URL into file_get_contents: if(substr($_GET['url'], 0, 4) != 'http') { exit; }

mootothemax 10y ago
> file_get_contents FYI, if you ever need a quick performance boost, switch to using curl for these calls. The difference is noticeable to the naked eye.
philo23 10y ago
If thats the only validation on calls to file_get_contents, that could very easily be bypassed. Entering something like just "/etc/passwd" for example.
- milankragujevic 10y ago
  /etc != http :)
  - philo23 10y ago
    Ah yes, you're right. Totally mis-read the code. It'd still leave access to any files in the same (or sub) directory starting with http, which realistically would probably be none but still something to bear in mind.
    - nitrogen 10y ago
      Right, best to check the whole URL. Something like "http/../../../etc/passwd" might get through otherwise.
      - milankragujevic 10y ago
        Like.... $url = rawurldecode($_GET['url']); $url_without_protocol = str_replace(array('https://', 'http://'), '', $url); $protocol = (stristr($url, 'https://') ? 'https' : 'http'); $page = file_get_contents($protocol . '://' . $url_without_protocol);

greenleafjacob 10y ago

This is the confused deputy problem. The most general solution to this class of vulnerabilities, SELinux, has been largely ignored. Does SELinux need more work to "bring it to market", or is it just too complicated and needs to be simplified?

pawadu 10y ago
configuring SELinux is way too complicated for the average user.
- nerdponx 10y ago
  And poorly documented. I learned what little I know about it from online tutorials, not the docs
- zanny 10y ago
  Then use one of the nowadays many alternatives - there is Apparmor, Tomoyo, and GRSec's RBAC all performing the same MAC job.
willvarfar 10y ago
SELinux is a real burden for even motivated sysadmins. However, if you have a single image that you are going to make millions of copies of then the effort vs reward might slant in SELinux's favour, e.g. Android does use SELinux.
rndgermandude 10y ago
SELinux is more the stopgap measure when everything else failed already, or at least it should be and prevent the most harmful things like reading random stuff from /etc. It is not something I'd say of "I got SELinux, now I don't need to validate user input". In the concrete example from the article, the process needs to access to /etc/hosts to do name resolutions, yet it should not send this information out to who knows who. How do you model that as a SELinux config? You cannot really. Unless you introduce dedicated (class of uncoupled, distinct, identifiable class of) processes acting as agents for resolving hosts with the help of /etc/hosts and whitelist them in SELinux... Which adds a whole lot of complexity. And you still have to make sure your new fancy agents cannot be tricked into giving up sensitive information. So at the end of the day, you should do defense in depth which of course should include user input validation and probably SELinux as well.

Grue3 10y ago

I tried Python requests and Common Lisp drakma, and neither of them can handle "file://" URL schema. Which HTTP client libraries are actually vulnerable to this?

simscitizen 10y ago
The most popular HTTP library is libcurl, and it supports file URLs by default.
jwilk 10y ago
Python's urllib(2), Perl's LWP
mapleoin 10y ago
urllib/urllib2's urlopen for example. I would assume it would be a bug for the requests library to allow file:///
konceptz 10y ago
Not to nitpick but it's not really a vulnerability. It's including the correct protocols for URIs.

Eridrus 10y ago

This is a special case of a Server-Side Request Forget vulnerability. Validating schemes is part of the answer but not the whole answer because attackers can still forge requests to internal resources you have firewalled off from the internet. These were recently released to help people deal with these issues since the details can be finicky: http://blog.includesecurity.com/2016/08/safeurl-server-side-request-forgery-protection-library.html http://blog.includesecurity.com/2016/08/safeurl-server-side-...

RegW 10y ago
Really. Surely if I tried to enter http://localhost:8983/solr/admin/cores?action=UNLOAD&core=core0&deleteIndex=true http://localhost:8983/solr/admin/cores?action=UNLOAD&core=co... the back service would be password protected. Oh, wait a minute.
Eridrus 10y ago
Should say "Server-Side Request Forgery", damn mobile.

jwilk 10y ago

> Actually the actual output all newlines had been stripped. Not stripped, but replaced by spaces. Also, the linked image looks like /etc/passwd, not /etc/hosts. > Weird. Not weird. That's how whitespace in HTML works.

kijin 10y ago
OP tested his hack on an online Markdown converter, so it probably has more to do with Markdown's treatment of whitespace than HTML's. As it happens, Markdown ignores most non-consecutive newlines.
- detaro 10y ago
  It's a converter taking HTML as input, converting it to Markdown.

wkd 10y ago

"Despite reporting the problem to the author on Friday, and following up the report via Twitter this has not yet been fixed, but after four days I assume I'm not alone in spotting this." Giving someone a weekend to fix something doesn't exactly sound like responsible disclosure. I understand if you get excited because you found a flaw but if you find something like this please be more responsible with publishing your findings.

kuschku 10y ago
Well, I found the same by pure chance before reading this article, and I suspect many more in the HN crowd did. If already half a dozen people on HN report they’ve found it and emailed the person about it, it’s likely it’s too late for responsible disclosure.
- 4ndr3vv 10y ago
  Agreed! Better get that karma before someone else does /snark
  - kuschku 10y ago
    The issue is more that if so many people have already found it, who else has?
    - 23andwalnut 10y ago
      Disclosing it publicly before it's been fixed only increases the number.
      - pg_is_a_butt 10y ago
        not disclosing it publicly when it's likely already known, only increases the number of victims as lack of pressure doesn't get the issue fixed immediately. am i doing it right? if you want to offer always on public services, then you have to have an always on company. always.

Bino 10y ago

There are others things to consider when fetching data from untrusted HTTP resources. A. Can it be used to DDoS B. How much data do you read (download, inbound may cost you real money)

r1ch 10y ago

Also be sure not to allow loopback connections (ie your own site or localhost) or you can cause a deadlock if the user requests the same URL to download from your site recursively. Choose an appropriate timeout too to prevent users tying up backend processes with a HTTP server that is slow to respond, and don't follow Location headers to avoid bypassing of your initial filters. A Range header should also be used to prevent users from telling your server to download multi-GB files and causing bandwidth waste / denial of service.

JoshTriplett 10y ago
> Also be sure not to allow loopback connections (ie your own site or localhost) or you can cause a deadlock if the user requests the same URL to download from your site recursively. Keep in mind that any arbitrary domain can point DNS to a loopback or LAN address. So the code that fetches a URL needs to include this filtering. > don't follow Location headers to avoid bypassing of your initial filters Often, you'll want to follow redirects, but re-apply your filters when doing so. > A Range header should also be used to prevent users from telling your server to download multi-GB files and causing bandwidth waste / denial of service. You can't count on support for Range, and the server might also just behave unexpectedly. Your fetching code needs to limit how much data it accepts from a server, and drop the connection after some upper bound.
- detaro 10y ago
  > Your fetching code needs to limit how much data it accepts from a server, and drop the connection after some upper bound. And if you accept compressed responses, remember that they can be a lot bigger when decompressed: e.g. gzip allows for factor 1000.

throwawayReply 10y ago

Also make sure you don't follow 301/302, or someone can set up a http link which redirects to file:// .

kuschku 10y ago
Or just, when following 301/302, call the same function again – which then validates the link completely again.
- throwawayReply 10y ago
  I'm not sure why you're getting downvoted, it's a little unfair for people to downvote a sensible suggestion without explaining why. One possible downside is that someone could Redirect A -> B and redirect B -> A, which risks tying up your resources following links, but browsers limit how many redirects will be followed, so it ought to be possible to limit redirects.
3pt14159 10y ago
Wait, what? Really!? Can this be used to get shell access somehow? I'm having trouble figuring out how you go from Chrome opening a remote file to _bad thing happens_.
- throwawayReply 10y ago
  It's not necessarily browsers, they often mitigate this. The problem is with services that consume other content. For example you might have a service which generates thumbnails of sites. That service might GET https://attacker.example.org/301.html https://attacker.example.org/301.html which itself might 301 back to file:///etc/passwd . If there is insufficient validation then a screenshot of the contents of /etc/passwd might be returned by the service. All of that happens outside the context of browsers and sandboxing. For more of that kind of thing, here's an interesting write up on some vulnerabilities found in Pocket. https://www.gnu.gl/blog/Posts/multiple-vulnerabilities-in-pocket/ https://www.gnu.gl/blog/Posts/multiple-vulnerabilities-in-po...
10y ago

majke 10y ago

file:// urls are awesome! People forget about them and nobody actually understands what to expect from these urls. Consider cross-origin policy implications. Is file:///home/john/bar.html on the same origin as file:///home/john/foo.html ? Also, XHR is totally allowed to go to file://

beilharz 10y ago

fuckyeahmarkdown fixed it just now.

byuu 10y ago

Also be very wary of ../ or possibly ..\ in URIs. Say you have http://example.org/download?file=release/software-1.0.zip http://example.org/download?file=release/software-1.0.zip so that you can log download statistics. If you just fetch GET["download"] and return it, you're gonna have a bad time if they try http://example.org/download?file=../../../../../etc/hosts http://example.org/download?file=../../../../../etc/hosts (browsers strip it out automatically, but it's easy enough to type such a request into a telnet session.) And don't just assume your safeguard works. Act like a hacker and try it out on yourself to make sure it works. Add an assert(validate("../") == false); at startup on your server, so it won't even run otherwise. Forbid the use of fopen() and instead go through your own file::open() function that calls validate() internally, then #define fopen ERROR_DONT_USE after (or whatever equivalent for the language you use.) It is a hostile world, you can never be too safe by adding multiple (even seemingly redundant) layers of protection.

shocks 10y ago
Also, you can't just find and replace ../ Consider: ....// which becomes ../
idbehold 10y ago
I've done something similar in node to ensure URIs like those cannot walk up past a base directory: function saferesolve(base, target) { var targetPath = '.' + path.posix.normalize('/' + target) return path.posix.resolve(base, targetPath) } saferesolve("./datasource", "a/b") === "./datasource/a/b" saferesolve("./datasource", "a/b/../c") === "./datasource/a/c" saferesolve("./datasource", "../..") === "./datasource" saferesolve("./datasource", "../../a/b") === "./datasource/a/b" saferesolve("./datasource", "../../a/b/..") === "./datasource/a"
- ajanuary 10y ago
  I've done something similar in Java, though more focused on file paths https://github.com/aJanuary/basepath https://github.com/aJanuary/basepath

konceptz 10y ago

Quick FYI. It's difficult to filter URIs with regex because they are context free grammars and belong to a larger language than regular expressions. Recursive descent parsers are the way to go like uri objects in most languages. Ex. URL url = URL.fromString(inputString); url.getScheme;

rybosome 10y ago

The vulnerability is happening because the web server is running as a user with elevated permissions. If it were running as a user who only had permissions to read files from the webserver's serving directory this would obviate the problem. Not that you shouldn't also validate input (in a whitelist rather than blacklist as others have said): just goes to show that security is a multifaceted problem with lots of ways to be paranoid. :)

nebulous1 10y ago
You don't need elevated privileges to read /etc/passwd
- daveloyall 10y ago
  That is correct. sebboh@namagiri:~$ ls -al /etc/passwd -rw-r--r-- 1 root root 1450 Jun 7 09:08 /etc/passwd So, there's this idea of running publicly facing services as a user which has less privileges than a normal interactive user. This user might be called 'nobody' or 'apache', etc. However, on your average distribution, /etc/passwd is accessible to the 'nobody' user or the 'apache' user. There are various work arounds: + run your service in a chroot or jail. + employ some kind of SE Linux thing. + ...? I've only ever used the first one.
  - delinka 10y ago
    Since ages now: https://en.wikipedia.org/wiki/Shadow_password https://en.wikipedia.org/wiki/Shadow_password
    - daveloyall 10y ago
      That doesn't mean you should allow your app to access `file://` ... (Nor allow it out of a chroot.) Simply knowing some usernames on your system could provide an attacker with clues...

kazinator 10y ago

Don't write server code that opens URI's that come in as input, period. If you take URI's as input, do it only to turn them around and spit them out into some Javascript sent back to the same session. Whatever can or cannot be accessed this way is the browser's problem.

elmigranto 10y ago
This works as long as your business model doesn't have opening user's URIs at its core, e.g. monitoring service.

homakov 10y ago

Also, if you're using ruby's open-uri: http://sakurity.com/blog/2015/02/28/openuri.html http://sakurity.com/blog/2015/02/28/openuri.html

ara24 10y ago

This is no different than failing to validate input parameters, and letting an attacker take a dump of your database. A service connecting to a remote location should have checked the URI scheme.

ppierald 10y ago

The u= parameter in the OPs article also is vulnerable (even if http/https are whitelisted, file:// blacklisted, etc) to the #10 vulnerability on the OWASP Top Ten 2013 list, namely Unvalidated Rediredcts and Forwards. https://www.owasp.org/index.php/Top_10_2013-A10-Unvalidated_Redirects_and_Forwards https://www.owasp.org/index.php/Top_10_2013-A10-Unvalidated_... I can easily get you to click the link to drive-by malware, adult sites, pharma, phishing, etc. because the site doesn't ensure where the link is actually going to.

_RPM 10y ago

a blacklist is generally not a good idea in any case. You should use a whitelist instead.

int_19h 10y ago

I think handling (rejecting) this in the libraries that handle URIs would be the wrong fix. While this is a vulnerability in a web app, it is a handy feature in an app that runs locally, and the library doesn't really know the context. It's up to the app to filter the URLs according to its isolation requirements.

phibit 10y ago

Another fun one that's becoming more common with cloud hosted services: access to internal metadata services that issue credentials. Blocking file:// doesn't prevent access from > http://169.254.169.254/latest/meta-data/iam/security-credentials/.. http://169.254.169.254/latest/meta-data/iam/security-credent....