Skip to content
New issue

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

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

Already on GitHub? # to your account

[vulnerability][acl] applyAcl is skipped for 2nd and later Object.assign sources if the 1st source is undefined #341

Closed
t2ym opened this issue Mar 3, 2020 · 0 comments

Comments

@t2ym
Copy link
Owner

t2ym commented Mar 3, 2020

[vulnerability][acl] applyAcl is skipped for 2nd and later Object.assign sources if the 1st source is undefined

Root Cause

  • Checking only the 1st source object

Reproducible Code

Object.assign({}, undefined, window)

Fix

  • Check all source objects even when sources include falsy values
diff --git a/demo/hook-callback.js b/demo/hook-callback.js
index 7129853d..cfbc2887 100644
--- a/demo/hook-callback.js
+++ b/demo/hook-callback.js
@@ -5713,10 +5713,13 @@ else {
                       property = _p;
                       break;
                     case S_TARGETED:
-                      if (_args[1][1] instanceof Object || (_args[1][1] && typeof _args[1][1] === 'object')) {
-                        rawProperty = [];
-                        for (let i = 1; i < _args[1].length; i++) {
-                          let _obj = _args[1][i];
+                      rawProperty = [];
+                      for (let i = 1; i < _args[1].length; i++) {
+                        let _obj = _args[1][i];
+                        if (!_obj) {
+                          continue;
+                        }
+                        if (_obj instanceof Object || typeof _obj === 'object') {
                           let _name = _globalObjects.get(_obj);
                           let _isStatic = true;
                           let _isObject = false;
@@ -5730,8 +5733,8 @@ else {
                           // TODO: Are inherited properties targeted?
                           rawProperty = rawProperty.concat(Object.keys(_args[1][i]));
                         }
-                        property = rawProperty.map(p => _escapePlatformProperties.get(p) || p);
                       }
+                      property = rawProperty.map(p => _escapePlatformProperties.get(p) || p);
                       break;
                     case S_ALL:
                       property = _p;
@@ -7223,10 +7226,13 @@ else {
                       property = _p;
                       break;
                     case S_TARGETED:
-                      if (_args[1][1] instanceof Object || (_args[1][1] && typeof _args[1][1] === 'object')) {
-                        rawProperty = [];
-                        for (let i = 1; i < _args[1].length; i++) {
-                          let _obj = _args[1][i];
+                      rawProperty = [];
+                      for (let i = 1; i < _args[1].length; i++) {
+                        let _obj = _args[1][i];
+                        if (!_obj) {
+                          continue;
+                        }
+                        if (_obj instanceof Object || typeof _obj === 'object') {
                           let _name = _globalObjects.get(_obj);
                           let _isStatic = true;
                           let _isObject = false;
@@ -7240,8 +7246,8 @@ else {
                           // TODO: Are inherited properties targeted?
                           rawProperty = rawProperty.concat(Object.keys(_args[1][i]));
                         }
-                        property = rawProperty.map(p => _escapePlatformProperties.get(p) || p);
                       }
+                      property = rawProperty.map(p => _escapePlatformProperties.get(p) || p);
                       break;
                     case S_ALL:
                       property = _p;

Note

  • 'string' sources are allowed in Object.assign() but their properties are not handled with ACL for now
console.log(Object.assign({}, 'abc')); 
{ 0: 'a', 1: 'b', 2: 'c' }
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant