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][acl] ACL is skipped for globals defined via defineProperty() and alike #325

Closed
t2ym opened this issue Feb 4, 2020 · 0 comments

Comments

@t2ym
Copy link
Owner

t2ym commented Feb 4, 2020

[vulnerability][acl] ACL is skipped for globals defined via defineProperty() and alike

Root Causes

  • Detected object name for the global object is checked against strings "self" and "window" although name is a Set instance for {"window"}
  • Extended global objects like Object.create(window) are not identified as global namespace objects

Reproducible Code

Object.assign(window, { Class: class Class {} });
new Class();

Object.defineProperty(window, 'DefinePropertyGlobalClass', 
{ value: class DefinePropertyGlobalClass {} });
new DefinePropertyGlobalClass();

let C = class DefinePropertyGetterGlobalClass {};
Object.defineProperty(window, 'DefinePropertyGetterGlobalClass', 
{ get: function () { return C; } });
new DefinePropertyGetterGlobalClass();

Object.defineProperty(window, 'DefinePropertyGetterVolatileGlobalClass', 
{ get: function () { return class DefinePropertyGetterVolatileGlobalClass { }; } });
new DefinePropertyGetterVolatileGlobalClass();

Object.defineProperty(window, 'DefinePropertyGetterReflectGetGlobalClass',
{ get: function () { return class DefinePropertyGetterReflectGetGlobalClass { }; } });
new (Reflect.get(window, 'DefinePropertyGetterReflectGetGlobalClass'))();

Object.defineProperty(window, 'DefinePropertyGetterReflectGetExtendedGlobalClass',
{ get: function () { return class DefinePropertyGetterReflectGetExtendedGlobalClass { }; }});
new (Reflect.get(Object.create(window),
'DefinePropertyGetterReflectGetExtendedGlobalClass'))();

Object.defineProperty(window, 'DefinePropertyGetterReflectGetExtendedGlobalClassWithReceiver', 
{ get: function () { return class DefinePropertyGetterReflectGetExtendedGlobalClassWithReceiver { }; }});
new (Reflect.get(Object.create(window), 'DefinePropertyGetterReflectGetExtendedGlobalClassWithReceiver', window))();

Object.defineProperties(window, { 'DefinePropertiesGlobalClass': { value: class DefinePropertiesGlobalClass {} } });
new DefinePropertiesGlobalClass();

let C2 = class DefinePropertiesGetterGlobalClass { };
Object.defineProperties(window, { 'DefinePropertiesGetterGlobalClass': 
{ get: function () { return C2; }} });
new DefinePropertiesGetterGlobalClass();

Object.defineProperties(window, { 'DefinePropertiesGetterVolatileGlobalClass': 
{ get: function () { return class DefinePropertiesGetterVolatileGlobalClass { }; } } });
new DefinePropertiesGetterVolatileGlobalClass();

Reflect.set(window, 'ReflectSetGlobalClass', class ReflectSetGlobalClass {});
new ReflectSetGlobalClass();

Fix Summary

  • Check whether normalizedThisArg === _global instead of switch (name) { case 'window' }
  • Check whether normalizedThisArg instanceof _global.constructor for Reflect.get(Object.create(window), 'prop'
  • Check whether receiver === _global for Reflect.get(Object.create(window),'prop',window)
  • Store the name of the global variable in trackResultAsGlobal for window.prop and Reflect.get(window, 'prop')
    • The initial value of trackResultAsGlobal is set as S_GLOBAL Symbol to mean the value is not set
  • targetNormalizer for Reflect.get() is changed from 'r01' to r01v so that the operation can be detected as potential global variable access

Notes

  • In accessing global functions/classes, the target objects are first read and then invoked
  • Getter functions can return an object with unique identity for each call via a class expression
t2ym added a commit that referenced this issue Feb 4, 2020
t2ym added a commit that referenced this issue Feb 4, 2020
@t2ym t2ym closed this as completed in e40efd5 Feb 4, 2020
t2ym added a commit that referenced this issue Feb 7, 2020
…d the side effect of the changes in Issue #325
t2ym added a commit that referenced this issue Feb 7, 2020
…perty to avoid the side effect of the changes in Issue #325
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