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

[vulnerability] Incorrect context for accessing global objects except for the first time #263

Closed
t2ym opened this issue May 9, 2018 · 1 comment

Comments

@t2ym
Copy link
Owner

t2ym commented May 9, 2018

[vulnerability] Incorrect context for reading or writing global objects except for the first time

Root Cause

  • The correct context handed to $hook$.global() is NOT properly handed to callback __hook__ function except for the first time read/write/call access to the target global object since the getter/setter functions of the wrapper property window._p_globalObjectName are bound to the scope for the first context which accessed the object and thus the context for the first access is used for ACL
    proxyDescriptor = Object.getOwnPropertyDescriptor(proxy, _hookPrefix + name);
    if (!proxyDescriptor) {
      // not accessed after the first time access to the global object
      newProxyDescriptor = {
        configurable: false,
        enumerable: false,
        get: function () {
          // context is bound to the scope of the first access to _global[name]
          return hookCallback(strictModePrefix + '.', _global, [name], context);
        },
        set: function (value) {
          // context is bound to the scope of the first access to _global[name]
          hookCallback(strictModePrefix + '=', _global, [name, value], context);
        }
      };
    }

Fix

  • Use the current access context handed to $hook$.global() function in the getter/setter functions
    proxyDescriptor = Object.getOwnPropertyDescriptor(proxy, _hookPrefix + name);
    if (proxyDescriptor) {
      // store the current context to the getter/setter function objects
      proxyDescriptor.get.context = context;
      proxyDescriptor.get.strictModePrefix = strictModePrefix;
      proxyDescriptor.set.context = context;
      proxyDescriptor.set.strictModePrefix = strictModePrefix;
    }
    else {
      let get = function get() {
        return hookCallback(get.strictModePrefix + '.', _global, [name], get.context);
      };
      get.context = context;
      get.strictModePrefix = strictModePrefix;
      let set = function set(value) {
        return hookCallback(set.strictModePrefix + '=', _global, [name, value], set.context);
      };
      set.context = context;
      set.strictModePrefix = strictModePrefix;
      newProxyDescriptor = {
        configurable: false,
        enumerable: false,
        get: get,
        set: set,
      };
    }

Reproducible Code Example

/*  /path.js  */
// The first access to navigator
// hooked as $hook$.global('/path.js,ableToAccessNavigator'), which persists in the following access
let ableToAccessNavigator = navigator;
// The second access to navigator
// The correct context '/path.js,unableToAccessNavigagor' is NOT applied
let unableToAccessNavigator = navigator;
@t2ym t2ym closed this as completed in cedac1f May 9, 2018
t2ym added a commit that referenced this issue May 9, 2018
t2ym added a commit that referenced this issue May 9, 2018
@t2ym
Copy link
Owner Author

t2ym commented May 9, 2018

Notes for 0.1.9

  • Discard using disastrously slow Object.assign() and use fast property assignments instead
  • get/set.strictModePrefix are properly used

t2ym added a commit that referenced this issue May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant