Deno: Reconsider enforcing the use of HTTPS for module download

Created on 14 May 2020  Â·  13Comments  Â·  Source: denoland/deno

Deno is marketed as “A secure JavaScript and TypeScript runtime” and the philosophy behind the flags disabling syscalls seems to be “Deno default behavior prevents you from shooting yourself in the foot security wise”.

However, by default, Deno accepts to download libraries using http only. In other words, https is not enforced. There are two issues about this (https://github.com/denoland/deno/issues/1063 and https://github.com/denoland/deno/issues/1064).
The first one, has been dismissed with the argument that: [Deno] “should follow the browser conventions here - using a script tag to import http is allowed.” The other one is still open but has not had any activity in a couple months.

As long as a Deno process instantiates a module with an http import, a “man in the middle attack” is possible:

It means it is possible to tamper with the code of the module that is downloaded. Even if Deno has implemented integrity checks for modules, there are still some key attack vectors.

As I understand, there are still two cases when integrity checks are not enforced: The first download of a lib and the update of a lib. Moreover, even in the cases where integrity checks are enforced, someone could just prevent you from downloading this module, or do something even nastier: send garbage for an infinite time.
In this last case (according to a test I ran on 1.0.0-rc3), Deno will just be stuck forever downloading the module while seeing its RSS increase until something stops it (user or OS). In this case, any dependency (or sub dependency) could selectively prevent Deno apps from installing , reaching actually a Denial of Service case. (Service being, having Deno running the code).

Another attack: someone could eavesdrop on the module that is being downloaded. Eventually, Deno users will use authentication to download certain modules. Over http, the credentials will be transmitted in clear. It is noticeable that the popular (and very cool) package manager @yarnpkg has been having an issue with this (see CVE-2019-5448). It is very possible that someone will actually fill a CVE against Deno on that exact topic at one point.

All of that can be introduced by importing a library in https that would actually import another one using http only.

Overall, It’s pretty easy to fix. It should just “by default, disallow http for module resolution”. Maybe a flag could be used to re-enable it, following what I understand is the philosophy of the project regarding security flags. This would be a breaking change however.

I saw that following the prototype pollution events in the JavaScript world, it was decided to remove Object.prototype.__proto__ (see https://github.com/denoland/deno/pull/4341). Actually, most browsers still support __proto__ (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto).

Here, the Deno team did the right thing, giving up browser compatibility to increase the default security level of the runtime. This is exactly the opposite of the decision taken regarding downloading modules using http only.

IMO, this defeats Deno being “Secure” if browser compatibility is more important than security. Deno is not a web browser. Also, web browsers have benefited from a lot of features regarding security hardening. For instance, security headers https://owasp.org/www-project-secure-headers/. Most of this would not make sense in Deno but they are here to update these behaviors.

(edit) Summary for ease of archieving:

  • current version of the project might be holding an instance of CWE-310 and/or CWE-311 out of similarity with CVE-2019-5448
  • current version of the project might be holding an instance of CWE-770

Most helpful comment

Just to repeat the point touched on at the end of #1063: the current behaviour is _not_ the same as standard browser behaviour or conventions.

Browsers will allow you to import HTTP scripts only from HTTP pages, where your entire context is already insecure. HTTPS pages cannot import insecure HTTP scripts due to mixed content rules, because that would be a security risk, and it's only allowed in HTTP pages because it's moot there anyway.

Given that you treat the script context in Deno as initially secure & trusted, importing content should behave like a browser import from a secure context imo, and should reject plain HTTP requests.

This is especially true given that browsers are rapidly moving towards wholesale deprecation of plain HTTP anyway:

All 13 comments

Just to repeat the point touched on at the end of #1063: the current behaviour is _not_ the same as standard browser behaviour or conventions.

Browsers will allow you to import HTTP scripts only from HTTP pages, where your entire context is already insecure. HTTPS pages cannot import insecure HTTP scripts due to mixed content rules, because that would be a security risk, and it's only allowed in HTTP pages because it's moot there anyway.

Given that you treat the script context in Deno as initially secure & trusted, importing content should behave like a browser import from a secure context imo, and should reject plain HTTP requests.

This is especially true given that browsers are rapidly moving towards wholesale deprecation of plain HTTP anyway:

Many browser does not allow mixing http and https scripts, especially in https site. I think this problem is analogous to mixed content in browsers.

There could be a flag "--allow-mixed-content" or something, and by default it would not allow peer-dependencies to import http-urls.

How is this not a duplicate of #1064? You could have added input there.

@nayeemrmn #1064 is about "Prevent modules imported through https to internally import http modules". Here this is about disabling http altogether by default for module import even for first level modules. At process level.

@vdeturckheim 🤷‍♂️ An https module importing an http one is an actual problem. Specifying an http main module is opting in at CLI. I don't know how this is pointing out an inconsistency with the security flags which exist specifically to opt in to potentially shooting yourself somewhere.

There is a reason #1064 talks about specifically about downgrading to http even though the reasoning is exactly the same.

@nayeemrmn

I don't know how this is pointing out an inconsistency with the security flags which exist specifically to opt in to potentially shooting yourself somewhere.

well, importing something in http is shooting yourself in the foot. Having a default behavior to prevent it and have a flag to "opt in to potentially shooting yourself" would actually be to disable http by default and allow it behind a flag right?

Importing executable code through https means you let the owner of the remote server run code on your machine.

Importing executable code through http means you let anyone on the internet run code on your machine.

I don't know how this is pointing out an inconsistency with the security flags which exist specifically to opt in to potentially shooting yourself somewhere.

well, importing something in http is shooting yourself in the foot. Having a default behavior to prevent it and have a flag to "opt in to potentially shooting yourself" would actually be to disable http by default and allow it behind a flag right?

No. Every script that uses http has a clear http in the URL (again, assuming no downgrades!). Not every script that depends on malicious runtime behaviour has "malicious-script.ts" in the name. That's the difference and why we need the permission flags.

I think we reached a disagreement here. Right now, the project is aiming at browser compatibility without providing the same level of security expectations as browsers (some of them because they don't make sense). The issue is coming from this disalignment and there might be other similar problems because of that exact root cause.

Even if it was only about downgrading, the solution would actually be the same. Changing the default value and place a flag.

Right... so it's a duplicate other than an unnecessary (imo) extension with the same solution.

IMO, this is not a duplicate because #1064 is a subcase of this.

But this exact part about debating if two security issues that have the same solution are the same is probably taking us to a place where we don't discuss the real point: There are multiple attack vectors for which mitigation is not possible for the time being while being possible in browsers.

Right... so it's a duplicate other than an unnecessary (imo) extension with the same solution.

In any case, duplicates or not, this is a real security issue that Deno should take care off.

Discord Conversation

As said in the Discord, I see no point adding flags (i.e: --allow-hrtime) for "security reason" while this is not sorted.

I see no point adding flags (i.e: --allow-hrtime) for "security reason" while this is not sorted.

Again, this is a ridiculous point. You're contributing nothing.

Reading the last messages of the screenshot I am under the impression that this issue might not be considered in its whole or might be dismissed quickly.
I don't feel comfortable keeping it open and will close the issue.

I have updated the original message to include a small summarizing conclusion to ease potential future references in case these topics are discussed down the road.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

xueqingxiao picture xueqingxiao  Â·  3Comments

zugende picture zugende  Â·  3Comments

justjavac picture justjavac  Â·  3Comments

CruxCv picture CruxCv  Â·  3Comments

metakeule picture metakeule  Â·  3Comments