Deno's fetch implementation is insecure by default (fetch("file:///app/.env"))

[alexgleason]

This is a follow-up to the comments starting here: #11925 (comment)

The key point is that fetch allows accessing file URIs by default, eg fetch("file:///app/.env")

This is a major problem, because:

1. It is common use-case to pass an entire URL into fetch from untrusted input.
2. fetch is an HTTP client. There is no expectation that it should handle anything except network traffic.
3. Deno advertises itself as a secure sandbox, and most users don't even know that they have to mitigate this.
4. The priorities are wrong. Deno wants to replace Node.js. Companies running Deno in production do not want this.

In the meantime, I created the deno-safe-fetch module to mitigate some of these issues.

The Deno security model doesn't fix this

I need to --allow-read=.env in order for Deno to read my secrets from a file so my application will work. This problem impacts secret files which I HAVE authorized Deno to read.

As a developer, I would NOT pass untrusted input to Deno.readFile. The whole point of using Deno.readFile is that it's a separate interface for accessing the filesystem.

Responding to arguments

It was requested in #2150 is the 10th most 👍 open issue at the time of this writing.

Just because people on GitHub say they want this, does not mean they have thought it through or that it's a good idea.

Unless you already do filtering on the url that you pass to fetch, you are already vulnerable to classes of the attack you described

Saying "this is more vulnerable than you thought" is not an excuse for it to be vulnerable. That makes it even worse.

I think you have fundamentally misunderstood what the security model of the fetch API in Deno is.

Apparently so. Me and how many others? When it's easy to misunderstand how something should be used, that strikes me as a problem. Especially since users have no expectation that fetch should do anything except make network calls.

our behaviour is entirely compliant with the Fetch spec - grep "file: URLs are left as an exercise for the reader".

Just because you can do something, doesn't mean you should. It is not a good idea to do this on a webserver at all, which is the main way that Deno is used.

Your mitigation does not solve the problem by the way, because I can just have a DNS entry on a custom domain that points to 127.0.0.1, or CNAMEs to another internal endpoint, or anything else. You can not protect against localhost access attacks using text based filtering of the hostname.

Oof. So DNS resolution has to happen to fix accessing internal IPs. At least this is within the realm of problems a security engineer would normally expect. Nobody expects that fetch would access files.

this adds a layer to your onion

Yes, exactly. Security is built in layers. There are layers of insecurity on fetch right now.

@lucacasonato

[alexgleason]

I can just have a DNS entry on a custom domain that points to 127.0.0.1, or CNAMEs to another internal endpoint, or anything else. You can not protect against localhost access attacks using text based filtering of the hostname.

I researched this problem a lot, and it turns out to be not entirely true. There is such as thing as DNS rebinding protection that can prevent this. If you have DNS rebinding protection, then filtering the hostname is enough. This is at least a problem a sysadmin can solve, unlike accessing file URIs.

[haehgeuwhveaw]

what is the conclusion, any news about this issue?

[alexgleason]

The answer I got was that fetch is working as intended by allowing you to fetch files. I still strongly disagree, because to me it's like an early 2000s path traversal vuln, something you'd expect from PHP. And there is no good reason for it except that it's cute, so it's also just bloat. I was very surprised they allowed this. I continue to use deno-safe-fetch in my projects.

[alexgleason]

Update: the --env-file flag added to Deno in #20300 fixes basically all my problems. With this I can use --env-file --deny-read=.env and it actually works.

The problem before was that I needed to allow reads to .env for the dotenv package to work. Doing so would inadvertently allow fetch to read it with a file: URI, forcing me to sanitize fetch inputs instead of relying on Deno permissions.

I still think it's a potential footgun. But I am more sympathetic to the idea it's a Deno permissions issue as long as the permissions API can actually address it. As long as people do it the new idiomatic way with --env-file, everything is fine.