Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Angular 1.5.8 Ban #1000

Closed
kspearrin opened this issue Oct 19, 2016 · 81 comments

Comments

Projects
None yet
@kspearrin
Copy link

commented Oct 19, 2016

Describe the problem and steps to reproduce it:

Submit Web Extension API extension using Angular 1.5.8

What happened?

Linter throws banned error with linked article.

What did you expect to happen?

Better explanation.

Anything else we should know?

Hey guys, we've been developing our new extension (https://github.com/bitwarden/browser) for months now and are hoping to bring it to Firefox. It has been on Chrome for a month or so now and we have great demand for Firefox. After a 1 month long wait for a review by the addons team here we were asked to make a slight change to remove some minified libraries. Now we go to re-submit our extension and we see that we can no longer publish it due to a ban on Angular 1.x outright. Obviously this sucks for us since we've invested so much time developing it over the past few months and can't just simply remove Angular without starting all over.

The linked slideshare about why it has been banned (http://www.slideshare.net/x00mario/an-abusive-relationship-with-angularjs) is from December 2015 and gives no indication that this is still a vulnerability. It even says Google has resolved it.

We are using the latest versions on Angular (1.5.8) and had no plans to update to 2.x since it's still so fresh and Google claims it will continue to support the 1.x versions for a long time.

Can you please shed further light as to why 1.5.8 is now banned? Do we have any other options rather than re-write our entire extension all over again? Can a more detailed, cooperative review allow us to bypass this all out ban?

Thank you.

@kspearrin kspearrin referenced this issue Oct 19, 2016

Closed

Firefox Status #8

@tofumatt

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

Paging @wagnerand, if Google is still supporting this version and it's fixed this seems like we're being too aggressive.

@wagnerand

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

My information is that Google stopped supporting angular 1.x months ago. It is now a community driven project. Also, we have been made aware of exploits that are not part of the presentation.

@kspearrin

This comment has been minimized.

Copy link
Author

commented Oct 20, 2016

@tofumatt @wagnerand I have never read anything from the team that says they are no longer supporting 1.x. Here are some posts that I found that have quotes from the Google team regarding support:

http://stackoverflow.com/questions/37037251/angularjs-1-x-support-lifecycle
https://www.quora.com/When-Angular-1-X-will-become-deprecated

Looking at the GitHub repo shows that the top 3 contributors over the past month are all from the Angular team themselves still:

https://github.com/angular/angular.js/pulse/monthly

Angular 2 was only released 1 month ago so already dropping support for Angular 1.x seems too aggressive to me.

@tofumatt

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

I agree with @kspearrin.

I know @wagnerand has mentioned other exploits (seemingly) not public; have you disclosed them to the Angular team so they can fix them? Seems like this is a bit too aggressive.

@wagnerand

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

Unfortunately, we were not able to report them to angular as the security researcher who found them asked us to not share them.
The underlying issue is a design specific issue though. Even if public exploits can be fixed, there will most probably be always another way to circumvent the patches.

@tofumatt

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

Ouch, that sounds brutal that they wouldn't share the security exploits with the team 😢

Okay, we should update the docs with this extra info then; even if we can't disclose the vulnerability we can say that we know of one and don't feel comfortable with people running angular in webextensions.

@kspearrin

This comment has been minimized.

Copy link
Author

commented Oct 20, 2016

Is there any possible way for us to get around this ban? Are all parts of Angular affected? Organizations with large extension projects cannot react to new versions so quickly (especially the large overhaul that Angular 2 is). It would seem to me that you are going to block out a large number of developers from distributing their extensions on Firefox since using Angular 1.x is still being encouraged by Chrome in their web extension documentation today.


Follow-up edit since it seems this has blown up:

I want to clarify that I am not suggesting that we be allowed to publish the extension if it is indeed affected by some Angular vulnerability. Since there are no details about the mentioned vulnerability, I am suggesting that if it does not affect the parts of the framework that we are using, we may be granted some special exemption and get around the blanket ban.

@kspearrin kspearrin changed the title Angular 1.5.8 Angular 1.5.8 Ban Oct 20, 2016

@joepie91

This comment has been minimized.

Copy link

commented Oct 20, 2016

It's very disturbing that the vulnerabilities are not being reported to the Angular team. That's an important part of responsible disclosure...

@justjanne

This comment has been minimized.

Copy link

commented Oct 20, 2016

@joepie91 But might be a legal issue, if the researcher only released the information to the FF team under a strict non-disclosure agreement.

There’s nothing they can do about it in such a case (but the researcher seems shady for not reporting it to the source indeed)

@ian-a-hickey

This comment has been minimized.

Copy link

commented Oct 20, 2016

Angular is removing the expression sandbox as of 1.6. They are doing that because the sandbox (while never intended to be used for security) was being used for security. Researchers kept showing over and over that the sandbox was not secure enough. Eventually, they just removed the sandbox. Personally, I feel like the sandbox (while not perfect) was actually pretty effective. I actually submitted the last known sandbox escape for versions 1.5 which seemed to lead to the removal of the sandbox. I now wish I hadn't. http://angularjs.blogspot.com/2016/09/angular-16-expression-sandbox-removal.html , angular/angular.js#14939

@Pharylon

This comment has been minimized.

Copy link

commented Oct 20, 2016

  • Google still supports 1.x.
  • Even if they didn't, does Mozilla suddenly have a beef with community-driven projects???
  • "There's a super-secret vulnerability in one the most popular web frameworks ever, seriously, I can't tell you what it is but it's totally there" doesn't pass the smell test.
@mprobst

This comment has been minimized.

Copy link

commented Oct 20, 2016

Hi. I'm working on security for Angular 1. We've reached out to Mozilla and the security researcher on this and are trying to clarify what's going on. We believe this is likely a misunderstanding. Please stay tuned, we'll comment on this issue.

@joepie91

This comment has been minimized.

Copy link

commented Oct 20, 2016

@justjanne If that happened (and that's highly speculative at this point), that seems like the moment for Mozilla to take a stand and say "no, we will not sign an NDA on this". NDAs for vulnerability disclosure (other than embargoes from the vendor themselves) are highly unusual, and for very good reason...

@ihsw This is probably not the right venue for a lengthy discussion about this, but the idea of "responsible disclosure" is to get things fixed as quickly as possible with as little impact as possible. While full disclosure is sometimes the answer to that (particularly with troublesome vendors), it would be wrong to equate "full disclosure" and "responsible disclosure" as if they are one and the same thing.

@Macil

This comment has been minimized.

Copy link

commented Oct 20, 2016

My understanding is that Angular runs eval-like functions on the page DOM. When Angular is run from within an extension using a DOM controlled by the webpage, then the webpage can write code into its own DOM which Angular from within the extension will execute with its extension privileges. Making a general secure sandbox for this type of case is very difficult (see how complicated Google Caja is; is the Angular team going to go through that much effort purely to make Angular work securely in extensions?). They've probably put band-aids around a few specific escapes, but as long as eval is used on arbitrary text from the page DOM controlled by an attacker, the root cause isn't fixed and there's probably many escapes more possible.

The fact that they've removed the sandboxing entirely in 1.6 makes it seem like they've come to the same conclusion, and decided to stop giving the appearance they've solved the issue. Angular 1.x fundamentally isn't designed for running in (higher-privileged) extensions on a shared page DOM.

@e-oz

This comment has been minimized.

Copy link

commented Oct 20, 2016

@wagnerand

It is now a community driven project.

Please clarify one thing especially: all libs and frameworks, driven by community, are enough reason for ban, right?

@crislin2046

This comment has been minimized.

Copy link

commented Oct 20, 2016

You have a responsibility to the people to share the vulnerability with the Angular team. Following the prohibition of the security researcher, right or wrong, at the expense of sharing information that could protect people and companies from these vulnerabilities, really doesn't seem right at all. Ultimately, it's incorrect to justify it by saying "researcher told me to do it", you have to take responsibility for your choice not to share it. So I don't think it works to simply deflect the matter to it having being already decided by someone else. There are likely issues bearing on the sharing of this information, that you could contributed to discussing in order to work out the best way forward. To silence that discussion from happening, because someone told you not to -- doesn't seem to be in everyone's benefit here.

@mooman219

This comment has been minimized.

Copy link

commented Oct 20, 2016

[Casual note this was posted on HackerNews, therefore the comment influx]

edit: updated link to not point to the nintendo switch hacker news post 🎉

@justjanne

This comment has been minimized.

Copy link

commented Oct 20, 2016

@dosaygo-coder-0 Many NDAs even prohibit discussing that you are under an NDA in the first place. There has been nothing confirmed yet regarding why Mozilla says they can’t discuss the issue or tell anyone about it yet, so we shouldn’t jump to conclusions just yet.

@Macil

This comment has been minimized.

Copy link

commented Oct 20, 2016

@dosaygo-coder-0

You have a responsibility to the people to share the vulnerability with the Angular team.

The Angular team already decided that the fundamental issues of Angular within extensions were not in scope of the project: the sandbox was specifically described as not a defense mechanism and was entirely removed in 1.6.

@callahad

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

[Thanks @mooman219 - DevRel is aware and will pay attention to the HN thread. Also emailing some folks right now and flagging this for followup next week once I get back from a conference.]

@ghost

This comment has been minimized.

Copy link

commented Oct 20, 2016

@mooman219 Your hackernews link goes to the nintendo switch discussion not the firefox one

@joshmanders

This comment has been minimized.

Copy link

commented Oct 20, 2016

@whitef0x0

This comment has been minimized.

Copy link

commented Oct 20, 2016

This is ridiculous. Even if there is a large security vulnerability, the fact that a dev finds this out the hard way is counterproductive.

Mozilla should support Angular by disclosing vulnerabilities to the Angular team ASAP

@crislin2046

This comment has been minimized.

Copy link

commented Oct 20, 2016

@justjanne indeed we shouldn't jump to conclusions. No one knows if an NDA is the excuse here or not. So far the excuse is "researcher said so." So no need to assume NDA for now.

@AgentME assuming Angular team might not need to know it doesn't seem like a strong reason to prevent you from sharing it. Maybe you don't understand the extension scope view of that team, maybe you don't understand the vulnerabilities. So how can you say they don't need it for sure? And if there's a small chance, you must share it.

Way safer to discuss everything with the relevant people, and share it with them, and decide together, than assuming you know what's best before doing that and everything's clear. Just a general point.

With @whitef0x0 agree. I think the important point here is that in the absence of total clarity, one shouldn't assume there's no need to share them. Keeping Angular team in the dark isn't going to help anyone get secured. It's reasonable to assume the vulnerabilities would be useful to them, and they are the ones who can fix it, so it must be shared, and there doesn't seem a good reason to not share them. "NDA" or "researcher" are just excuses that limit discussion and deflect responsibility, delaying the actual result of a fix from happening.

@Macil

This comment has been minimized.

Copy link

commented Oct 20, 2016

@dosaygo-coder-0 The vulnerability was within a feature specifically described as not a security feature and was already removed in 1.6.

Angular runs eval on text in the page DOM. When the DOM is controlled by someone with lower privileges (ie. Angular is running in a higher-privileged extension with the DOM controlled by a webpage), then they can elevate their privileges by placing code in the DOM and letting Angular execute it within the extension.

Angular <1.6 blacklisted specific ways to do this, but it was not nearly complete. Fully securing this would take a ton of effort and would heavily bloat the size of Angular. The vulnerability mentioned in #1000 (comment) was most likely another way around this blacklist. The blacklist was not guaranteed or even intended to be foolproof: it's described here as "not a defense mechanism". The blacklist was entirely removed in 1.6. They admit that the sandbox blacklist was never enough to properly secure Angular in contexts such as extensions where an attacker can insert templates; from that post, it's apparent they're not interested in knowing more ways it could have been bypassed.

@dveditz

This comment has been minimized.

Copy link

commented Oct 20, 2016

I'm on the incoming end of most security bug submissions to Mozilla. We don't sign NDAs to get bug info. Besides, that would involve the lawyers and takes forever. We do sometimes agree to embargo a bug, especially if it involves multiple products (not withholding from those products, but so they have time to patch also). We definitely would not agree to withhold a vulnerability from the maintainers of an upstream library if that's where it needs to be patched.

@nikoTM

This comment has been minimized.

Copy link

commented Oct 20, 2016

@dveditz so how come this was not forwarded to angular team ?

@petebacondarwin

This comment has been minimized.

Copy link

commented Nov 18, 2016

Thanks for the review @Rob--W - I will talk with @mprobst in the USA morning and come back with a response.

@pankajkhanzode

This comment has been minimized.

Copy link

commented Nov 23, 2016

@petebacondarwin - Any update on progress made here? We have been waiting for weeks now, and while long term we plan to move to Angular 2.0, it is definitely not couple of days work to move there. So we are depending on your interaction and making Angular 1.5.* go ahead and get white-listed again. Thank you very much!

@mi-g

This comment has been minimized.

Copy link

commented Nov 23, 2016

Waiting here for the final call too, with a few million users depending on that decision.

@petebacondarwin

This comment has been minimized.

Copy link

commented Nov 23, 2016

I will have an update on changes to our patch if necessary by the close of US daytime today.

@IgorMinar

This comment has been minimized.

Copy link

commented Nov 24, 2016

Hello everyone, on behalf of the Angular team I'm posting an update after our meeting with the Mozilla folks today.

These were the key take aways from the meeting:

  • Mozilla folks agreed that to the change angular/angular.js#15346 we implemented 3 weeks ago is a good workaround for the problem identified as "Firefox script loader can be tricked into loading angular.js into websites from Firefox addons, where once angular autobootstraps bad things could happen"
    • Mozilla security folks requested a few minor tweaks to the implementation that we already implemented as angular/angular.js#15427 right after our meeting
  • Mozilla folks expressed concerns about the use case where addons inject Angular into web pages via a content script. The number of addons that do this is currently limited.
    • Angular team believes that this is an anti-pattern due to concerns about the impact of loading Angular into a foreign environment that could cause various unexpected collisions. We recommended that such addons are banned until the day when this kind of usage is officially supported by Angular - this is however currently not on our roadmap.
  • No other significant concerns were brought to our attention by the Mozilla team.
    • Angular team committed to release version 1.5.9 that will contain the two pull requests mentioned above tomorrow (Nov 24).
    • Mozilla team committed to lifting the ban for all version of Angular >=1.5.9 effective Dec 1, 2016 when they'll push an update to their addon linting infrastructure.

I'm hopeful that after weeks of delays, this issue will be finally resolved on December 1, 2016. In the meantime, happy Thanksgiving to those observing Thanksgiving tomorrow! 🦃

@deadsquid please confirm this agreement as the Mozilla representative. If I accidentally got anything wrong, please point it out. Thank you!

@pankajkhanzode

This comment has been minimized.

Copy link

commented Nov 24, 2016

Thanks a lot @IgorMinar and Angular team for resolving it with Mozilla team. Happy ThanksGiving!

@SebastianMetzger

This comment has been minimized.

Copy link

commented Nov 24, 2016

@IgorMinar This is very good news, I already made a calendar entry for a december 1st release for our team 💯
@deadsquid Please confirm 👍

@petebacondarwin

This comment has been minimized.

Copy link

commented Nov 24, 2016

We have just released 1.5.9 containing the security patches requested by Mozilla. It is available right now on npm and code.angularjs.org; it will be available on the Google CDN tomorrow.

We are also releasing 1.6.0-rc.2 containing the same patches later today.

We expect that Mozilla will lift its ban on add-ons built with Angular 1.5.9+ by December 1st.

@pankajkhanzode

This comment has been minimized.

Copy link

commented Nov 25, 2016

Thanks @petebacondarwin , @deadsquid, @IgorMinar and @mprobst + @Rob--W - for working together. 1.5.9 is great news!

@deadsquid

This comment has been minimized.

Copy link

commented Nov 25, 2016

Howdy folks,

We should have a review policy update out early next week and a push to the linter by end-of-day (Pacific time) on Dec 1st, per @IgorMinar's post above.

It's important to note a couple things:

  • If your add-on uses Angular, you will need to include unmodified, non-repackaged versions. Add-ons that incorporate modified versions will be rejected.
  • We'll be reaching out to affected add-on developers to update their extension to incorporate the patched version of Angular. Additional changes may be requested, and we'd ask that you keep an eye out for email from us and address any changes as quickly as possible.

Thanks again to the Angular team for working through the issues discovered with us, and for incorporating the additional changes.

(edited to let add-on developers know that we may ask for changes other than just upgrading)

@tofumatt tofumatt assigned tofumatt and unassigned wagnerand Nov 29, 2016

@tofumatt tofumatt closed this in fa83449 Nov 29, 2016

tofumatt added a commit that referenced this issue Nov 29, 2016

@andymckay

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2016

Just a note that this is scheduled to be deployed Dec 1st as per the AMO deployment schedule calendar: https://calendar.google.com/calendar/embed?src=mozilla.com_lr5jsh38i6dmr72uu4d1nv7dcc%40group.calendar.google.com

@cure53

This comment has been minimized.

Copy link

commented Feb 23, 2017

Bypassed the fix, wrote @petebacondarwin :)

@petebacondarwin

This comment has been minimized.

Copy link

commented Feb 23, 2017

Aw man!

@petebacondarwin

This comment has been minimized.

Copy link

commented Feb 24, 2017

@cure53 - we have landed fixes for the bypasses in master, 1.6 and 1.5 branches. See:

@kspearrin

This comment has been minimized.

Copy link
Author

commented Feb 24, 2017

@petebacondarwin What 1.5.x version can we target for this fix?

@cure53

This comment has been minimized.

Copy link

commented Feb 24, 2017

@petebacondarwin LGTM, @masatokinugawa what do you think?

@petebacondarwin

This comment has been minimized.

Copy link

commented Feb 24, 2017

@kspearrin - we were hoping not to do any more 1.5.x releases, but I guess we might have to make an exception in this case. The difficulty is that we must get 1.5.x commits synced into Google before we can release so it will take a little longer. The version number will be 1.5.12

@kspearrin

This comment has been minimized.

Copy link
Author

commented Feb 24, 2017

@petebacondarwin If 1.6.x is the only upgrade path that is fine, we'll just have to make that move sooner than originally planned. I was just under the impression you would be releasing 1.5.x as well since you mentioned the change was applied in that branch as well.

@masatokinugawa

This comment has been minimized.

Copy link

commented Feb 25, 2017

I found it still can be bypassed on a specific situation.
@petebacondarwin Shall I report details to pete@ba[...]com?

@andymckay

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2017

This issue is closed and emails quite a lot of people in the mozilla add-ons team. If you have other issues could you please report them to the angular team using their reporting system. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.