Fixed bypass for prototype pollution in baseExtend #17086

Asjidkalam · 2020-10-07T16:39:42Z

By blocking __proto__, prototype, and constructor on deep merging, this commit prevents the Object prototype from being polluted.

There is already mitigation for prototype pollution on #16913, but it only blocks the __proto__ key and it's bypassable using the prototype and constructor keys. This commit fixes the issue and completely mitigate the prototype pollution issue on deepMerge.

PoC for bypassing the current fix:

<!DOCTYPE html>
<html ng-app="">
  <head>
    <title>AngularJS Prototype pollution PoC</title>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.8.0/angular.js"></script>
  </head>
  <body>
    <fieldset>
      <legend>AngularJS Merge() - Prototype Pollution PoC</legend>
      <script>
        var src = JSON.parse('{ "prototype": { "polluted": "true" }}');
        var dst = {};
        angular.merge(dst, src);

        document.write(dst.prototype.polluted);
        console.log(dst);
      </script>
    </fieldset>
  </body>
</html>

AngularJS is in LTS mode

We are no longer accepting changes that are not critical bug fixes into this project.
See https://blog.angular.io/stable-angularjs-and-long-term-support-7e077635ee9c for more detail.

Does this PR fix a regression since 1.7.0, a security flaw, or a problem caused by a new browser version?

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?
Just extended the old fix.

Does this PR introduce a breaking change?
No breaking changes introduced.

Please check if the PR fulfills these requirements

The commit message follows our guidelines
Fix/Feature: Docs have been added/updated
Fix/Feature: Tests have been added; existing tests pass

Other information:

google-cla · 2020-10-07T16:39:47Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

Asjidkalam · 2020-10-07T16:41:27Z

@googlebot I signed it!

gkalpak · 2020-10-10T21:06:44Z

Thx for submitting a PR, @Asjidkalam. However, I'm afraid there is some confusion around the fix in #16913.

The issue fixed in #16913 was that Object.prototype could be modified by running angular.merge() on an arbitrary object. See this test for an illustration.

Neither prototype nor constructor are affected by the same issue. I.e. the only way to affect all objects is by calling angular.merge() on Object or Object.prototype, in which case the changes are intentional.

I am going to close this, but feel free to continue the discussion below.

Fixed bypass for prototype pollution in baseExtend

aa7c1b0

google-cla bot added the cla: no label Oct 7, 2020

google-cla bot added cla: yes and removed cla: no labels Oct 7, 2020

gkalpak closed this Oct 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed bypass for prototype pollution in baseExtend #17086

Fixed bypass for prototype pollution in baseExtend #17086

Asjidkalam commented Oct 7, 2020

google-cla bot commented Oct 7, 2020

Asjidkalam commented Oct 7, 2020

gkalpak commented Oct 10, 2020

Fixed bypass for prototype pollution in baseExtend #17086

Fixed bypass for prototype pollution in baseExtend #17086

Conversation

Asjidkalam commented Oct 7, 2020

AngularJS is in LTS mode

google-cla bot commented Oct 7, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

Asjidkalam commented Oct 7, 2020

gkalpak commented Oct 10, 2020