Skip to content
Permalink
Browse files Browse the repository at this point in the history
Harden Snowboard (#687)
- The Snowboard and PluginLoader objects are now frozen and cannot be modified.
- Added a Proxy in front of Snowboard to handle plugin loading
- Plugin "Snowboard" instances are blocked from running certain methods
- Update tests to check hardening
  • Loading branch information
bennothommo committed Sep 13, 2022
1 parent e695dd8 commit 2a13faf
Show file tree
Hide file tree
Showing 17 changed files with 392 additions and 49 deletions.
6 changes: 5 additions & 1 deletion modules/system/.eslintrc.json
Expand Up @@ -33,5 +33,9 @@
"math": "always"
}],
"vue/multi-word-component-names": ["off"]
}
},
"ignorePatterns": [
"tests/js",
"**/build/*.js"
]
}
2 changes: 1 addition & 1 deletion modules/system/assets/js/build/system.js

Large diffs are not rendered by default.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion modules/system/assets/js/snowboard/build/snowboard.base.js

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

43 changes: 43 additions & 0 deletions modules/system/assets/js/snowboard/main/InnerProxyHandler.js
@@ -0,0 +1,43 @@
/**
* Internal proxy for Snowboard.
*
* This handler wraps the Snowboard instance that is passed to the constructor of plugin instances.
* It prevents access to the following methods:
* - `attachAbstracts`: No need to attach abstracts again.
* - `loadUtilties`: No need to load utilities again.
* - `initialise`: Snowboard is already initialised.
* - `initialiseSingletons`: Singletons are already initialised.
*/
export default {
get(target, prop, receiver) {
if (typeof prop === 'string') {
const propLower = prop.toLowerCase();

if (['attachAbstracts', 'loadUtilities', 'initialise', 'initialiseSingletons'].includes(prop)) {
throw new Error(`You cannot use the "${prop}" Snowboard method within a plugin.`);
}

if (target.hasPlugin(propLower)) {
return (...params) => Reflect.get(target, 'plugins')[propLower].getInstance(...params);
}
}

return Reflect.get(target, prop, receiver);
},

has(target, prop) {
if (typeof prop === 'string') {
const propLower = prop.toLowerCase();

if (['attachAbstracts', 'loadUtilities', 'initialise', 'initialiseSingletons'].includes(prop)) {
return false;
}

if (target.hasPlugin(propLower)) {
return true;
}
}

return Reflect.has(target, prop);
},
};
30 changes: 25 additions & 5 deletions modules/system/assets/js/snowboard/main/PluginLoader.js
@@ -1,5 +1,6 @@
import PluginBase from '../abstracts/PluginBase';
import Singleton from '../abstracts/Singleton';
import InnerProxyHandler from './InnerProxyHandler';

/**
* Plugin loader class.
Expand All @@ -22,13 +23,28 @@ export default class PluginLoader {
*/
constructor(name, snowboard, instance) {
this.name = name;
this.snowboard = snowboard;
this.snowboard = new Proxy(
snowboard,
InnerProxyHandler,
);
this.instance = instance;

// Freeze instance that has been inserted into this loader
Object.freeze(this.instance);

this.instances = [];
this.singleton = instance.prototype instanceof Singleton;
this.initialised = instance.prototype instanceof PluginBase;
this.singleton = {
initialised: false,
};
// Prevent further extension of the singleton status object
Object.seal(this.singleton);

this.mocks = {};
this.originalFunctions = {};

// Freeze loader itself
Object.freeze(PluginLoader.prototype);
Object.freeze(this);
}

/**
Expand Down Expand Up @@ -162,7 +178,11 @@ export default class PluginLoader {
* @returns {boolean}
*/
isInitialised() {
return this.initialised;
if (!this.isSingleton()) {
return true;
}

return this.singleton.initialised;
}

/**
Expand All @@ -179,7 +199,7 @@ export default class PluginLoader {
newInstance.detach = () => this.instances.splice(this.instances.indexOf(newInstance), 1);
newInstance.construct(...parameters);
this.instances.push(newInstance);
this.initialised = true;
this.singleton.initialised = true;
}

/**
Expand Down
25 changes: 25 additions & 0 deletions modules/system/assets/js/snowboard/main/ProxyHandler.js
@@ -0,0 +1,25 @@
export default {
get(target, prop, receiver) {
if (typeof prop === 'string') {
const propLower = prop.toLowerCase();

if (target.hasPlugin(propLower)) {
return (...params) => Reflect.get(target, 'plugins')[propLower].getInstance(...params);
}
}

return Reflect.get(target, prop, receiver);
},

has(target, prop) {
if (typeof prop === 'string') {
const propLower = prop.toLowerCase();

if (target.hasPlugin(propLower)) {
return true;
}
}

return Reflect.has(target, prop);
},
};
35 changes: 23 additions & 12 deletions modules/system/assets/js/snowboard/main/Snowboard.js
Expand Up @@ -31,9 +31,17 @@ export default class Snowboard {
this.plugins = {};
this.listeners = {};
this.foundBaseUrl = null;
this.domReady = false;

this.readiness = {
dom: false,
};
// Seal readiness from being added to further, but allow the properties to be modified.
Object.seal(this.readiness);
this.attachAbstracts();

// Freeze the Snowboard class to prevent further modifications.
Object.freeze(Snowboard.prototype);
Object.freeze(this);

this.loadUtilities();
this.initialise();

Expand All @@ -55,6 +63,11 @@ export default class Snowboard {
attachAbstracts() {
this.PluginBase = PluginBase;
this.Singleton = Singleton;

Object.freeze(this.PluginBase.prototype);
Object.freeze(this.PluginBase);
Object.freeze(this.Singleton.prototype);
Object.freeze(this.Singleton);
}

/**
Expand All @@ -79,7 +92,7 @@ export default class Snowboard {
this.initialiseSingletons();
}
this.globalEvent('ready');
this.domReady = true;
this.readiness.dom = true;
});
}

Expand Down Expand Up @@ -124,9 +137,6 @@ export default class Snowboard {
}

this.plugins[lowerName] = new PluginLoader(lowerName, this, instance);
const callback = (...parameters) => this.plugins[lowerName].getInstance(...parameters);
this[name] = callback;
this[lowerName] = callback;

this.debug(`Plugin "${name}" registered`);

Expand All @@ -139,7 +149,7 @@ export default class Snowboard {
&& plugin.dependenciesFulfilled()
&& plugin.hasMethod('listens')
&& Object.keys(plugin.callMethod('listens')).includes('ready')
&& this.domReady
&& this.readiness.dom
) {
const readyMethod = plugin.callMethod('listens').ready;
plugin.callMethod(readyMethod);
Expand Down Expand Up @@ -213,11 +223,13 @@ export default class Snowboard {
* @returns {PluginLoader}
*/
getPlugin(name) {
if (!this.hasPlugin(name)) {
throw new Error(`No plugin called "${name}" has been registered.`);
const lowerName = name.toLowerCase();

if (!this.hasPlugin(lowerName)) {
throw new Error(`No plugin called "${lowerName}" has been registered.`);
}

return this.plugins[name];
return this.plugins[lowerName];
}

/**
Expand Down Expand Up @@ -263,7 +275,7 @@ export default class Snowboard {
* @param {Function} callback
*/
ready(callback) {
if (this.domReady) {
if (this.readiness.dom) {
callback();
}

Expand Down Expand Up @@ -524,7 +536,6 @@ export default class Snowboard {
this.logMessage('rgb(45, 167, 199)', false, message, ...parameters);
}


/**
* Log a debug message.
*
Expand Down
6 changes: 5 additions & 1 deletion modules/system/assets/js/snowboard/snowboard.base.debug.js
@@ -1,7 +1,11 @@
import Snowboard from './main/Snowboard';
import ProxyHandler from './main/ProxyHandler';

((window) => {
const snowboard = new Snowboard(true, true);
const snowboard = new Proxy(
new Snowboard(true, true),
ProxyHandler,
);

// Cover all aliases
window.snowboard = snowboard;
Expand Down
6 changes: 5 additions & 1 deletion modules/system/assets/js/snowboard/snowboard.base.js
@@ -1,7 +1,11 @@
import Snowboard from './main/Snowboard';
import ProxyHandler from './main/ProxyHandler';

((window) => {
const snowboard = new Snowboard();
const snowboard = new Proxy(
new Snowboard(),
ProxyHandler,
);

// Cover all aliases
window.snowboard = snowboard;
Expand Down
97 changes: 97 additions & 0 deletions modules/system/tests/js/cases/snowboard/main/PluginLoader.test.js
Expand Up @@ -34,4 +34,101 @@ describe('PluginLoader class', function () {
}
);
});

it('is frozen on construction and doesn\'t allow prototype pollution', function () {
FakeDom
.new()
.addScript([
'modules/system/assets/js/build/manifest.js',
'modules/system/assets/js/snowboard/build/snowboard.vendor.js',
'modules/system/assets/js/snowboard/build/snowboard.base.js',
])
.render()
.then(
(dom) => {
const loader = dom.window.Snowboard.getPlugin('sanitizer');

expect(() => {
loader.newMethod = () => {
return true;
};
}).toThrow(TypeError);

expect(() => {
loader.newProperty = 'test';
}).toThrow(TypeError);

expect(() => {
loader.singleton.test = 'test';
}).toThrow(TypeError);

expect(loader.newMethod).toBeUndefined();
expect(loader.newProperty).toBeUndefined();
},
(error) => {
throw error;
}
);
});

it('should prevent modification of root instances', function () {
FakeDom
.new()
.addScript([
'modules/system/assets/js/build/manifest.js',
'modules/system/assets/js/snowboard/build/snowboard.vendor.js',
'modules/system/assets/js/snowboard/build/snowboard.base.js',
'modules/system/tests/js/fixtures/framework/TestPlugin.js',
'modules/system/tests/js/fixtures/framework/TestSingleton.js',
])
.render()
.then(
(dom) => {
const rootInstance = dom.window.Snowboard.getPlugin('testPlugin').instance;

expect(() => {
rootInstance.newMethod = () => {
return true;
}
}).toThrow(TypeError);

expect(rootInstance.newMethod).toBeUndefined();

// Modifications can however be made to instances retrieved from the loader
const loadedInstance = dom.window.Snowboard.getPlugin('testPlugin').getInstance();

loadedInstance.newMethod = () => {
return true;
};
expect(loadedInstance.newMethod).toEqual(expect.any(Function));
expect(loadedInstance.newMethod()).toBe(true);

// But shouldn't follow through to new instances
const loadedInstanceTwo = dom.window.Snowboard.getPlugin('testPlugin').getInstance();
expect(loadedInstanceTwo.newMethod).toBeUndefined();

// The same rules apply for singletons, except that modifications will follow through to other uses
// of the singleton, since it's only one global instance.
const rootSingleton = dom.window.Snowboard.getPlugin('testSingleton').instance;

expect(() => {
rootSingleton.newMethod = () => {
return true;
}
}).toThrow(TypeError);

const loadedSingleton = dom.window.Snowboard.getPlugin('testSingleton').getInstance();

loadedSingleton.newMethod = () => {
return true;
};
expect(loadedSingleton.newMethod).toEqual(expect.any(Function));
expect(loadedSingleton.newMethod()).toBe(true);

const loadedSingletonTwo = dom.window.Snowboard.getPlugin('testSingleton').getInstance();
expect(loadedSingletonTwo.newMethod).toEqual(expect.any(Function));
expect(loadedSingletonTwo.newMethod()).toBe(true);
}
);
});
});

0 comments on commit 2a13faf

Please sign in to comment.