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

modules/system/.eslintrc.json

@@ -33,5 +33,9 @@
             "math": "always"
         }],
         "vue/multi-word-component-names": ["off"]
-    }
+    },
+    "ignorePatterns": [
+        "tests/js",
+        "**/build/*.js"
+    ]
 }

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);
+    },
+};

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.
@@ -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);
     }
 
     /**
@@ -162,7 +178,11 @@ export default class PluginLoader {
      * @returns {boolean}
      */
     isInitialised() {
-        return this.initialised;
+        if (!this.isSingleton()) {
+            return true;
+        }
+
+        return this.singleton.initialised;
     }
 
     /**
@@ -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;
     }
 
     /**

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);
+    },
+};

modules/system/assets/js/snowboard/main/Snowboard.js

@@ -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();
 
@@ -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);
     }
 
     /**
@@ -79,7 +92,7 @@ export default class Snowboard {
                 this.initialiseSingletons();
             }
             this.globalEvent('ready');
-            this.domReady = true;
+            this.readiness.dom = true;
         });
     }
 
@@ -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`);
 
@@ -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);
@@ -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];
     }
 
     /**
@@ -263,7 +275,7 @@ export default class Snowboard {
      * @param {Function} callback
      */
     ready(callback) {
-        if (this.domReady) {
+        if (this.readiness.dom) {
             callback();
         }
 
@@ -524,7 +536,6 @@ export default class Snowboard {
         this.logMessage('rgb(45, 167, 199)', false, message, ...parameters);
     }
 
-
     /**
      * Log a debug message.
      *

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;

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;

modules/system/tests/js/cases/snowboard/main/PluginLoader.test.js

@@ -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);
+                }
+            );
+    });
 });

modules/system/tests/js/cases/snowboard/main/Snowboard.test.js

@@ -42,6 +42,49 @@ describe('Snowboard framework', 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',
+                'modules/system/tests/js/fixtures/framework/TestPlugin.js',
+            ])
+            .render()
+            .then(
+                (dom) => {
+                    expect(() => {
+                        dom.window.Snowboard.newMethod = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.newProperty = 'test';
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.readiness.test = 'test';
+                    }).toThrow(TypeError);
+
+                    expect(dom.window.Snowboard.newMethod).toBeUndefined();
+                    expect(dom.window.Snowboard.newProperty).toBeUndefined();
+
+                    // You should not be able to modify the Snowboard object fed to plugins either
+                    const instance = dom.window.Snowboard.testPlugin();
+                    expect(() => {
+                        instance.snowboard.newMethod = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+                },
+                (error) => {
+                    throw error;
+                }
+            );
+    });
+
     it('can add and remove a plugin', function (done) {
         FakeDom
             .new()
@@ -59,16 +102,22 @@ describe('Snowboard framework', function () {
 
                     try {
                         // Check plugin caller
-                        expect(Snowboard.hasPlugin('test')).toBe(true);
-                        expect(Snowboard.getPluginNames()).toEqual(
-                            expect.arrayContaining(['jsonparser', 'sanitizer', 'test'])
-                        );
-                        expect(Snowboard.test).toEqual(expect.any(Function));
+                        expect('testPlugin' in Snowboard).toEqual(true);
+                        expect('testSingleton' in Snowboard).toEqual(false);
 
-                        const instance = Snowboard.test();
+                        expect(Snowboard.hasPlugin('testPlugin')).toBe(true);
+                        expect(Snowboard.getPluginNames()).toEqual(
+                            expect.arrayContaining(['jsonparser', 'sanitizer', 'testplugin'])
+                        );
+
+                        const instance = Snowboard.testPlugin();
 
                         // Check plugin injected methods
-                        expect(instance.snowboard).toBe(Snowboard);
+                        expect(instance.snowboard).toBeDefined();
+                        expect(instance.snowboard.getPlugin).toEqual(expect.any(Function));
+                        expect(() => {
+                            const method = instance.snowboard.initialise;
+                        }).toThrow('cannot use');
                         expect(instance.destructor).toEqual(expect.any(Function));
 
                         // Check plugin method
@@ -77,20 +126,20 @@ describe('Snowboard framework', function () {
                         expect(instance.testMethod()).toEqual('Tested');
 
                         // Check multiple instances
-                        const instanceOne = Snowboard.test();
+                        const instanceOne = Snowboard.testPlugin();
                         instanceOne.changed = true;
-                        const instanceTwo = Snowboard.test();
+                        const instanceTwo = Snowboard.testPlugin();
                         expect(instanceOne).not.toEqual(instanceTwo);
-                        const factory = Snowboard.getPlugin('test');
+                        const factory = Snowboard.getPlugin('testPlugin');
                         expect(factory.getInstances()).toEqual([instance, instanceOne, instanceTwo]);
 
                         // Remove plugin
-                        Snowboard.removePlugin('test');
-                        expect(Snowboard.hasPlugin('test')).toEqual(false);
+                        Snowboard.removePlugin('testPlugin');
+                        expect(Snowboard.hasPlugin('testPlugin')).toEqual(false);
                         expect(dom.window.Snowboard.getPluginNames()).toEqual(
                             expect.arrayContaining(['jsonparser', 'sanitizer'])
                         );
-                        expect(Snowboard.test).not.toBeDefined();
+                        expect(Snowboard.testPlugin).not.toBeDefined();
 
                         done();
                     } catch (error) {
@@ -119,17 +168,24 @@ describe('Snowboard framework', function () {
                     const Snowboard = dom.window.Snowboard;
 
                     try {
-                         // Check plugin caller
-                        expect(Snowboard.hasPlugin('test')).toBe(true);
-                        expect(Snowboard.getPluginNames()).toEqual(
-                            expect.arrayContaining(['jsonparser', 'sanitizer', 'test'])
-                        );
-                        expect(Snowboard.test).toEqual(expect.any(Function));
+                        expect('testPlugin' in Snowboard).toEqual(false);
+                        expect('testSingleton' in Snowboard).toEqual(true);
 
-                        const instance = Snowboard.test();
+                         // Check plugin caller
+                        expect(Snowboard.hasPlugin('testSingleton')).toBe(true);
+                        expect(Snowboard.getPluginNames()).toEqual(
+                            expect.arrayContaining(['jsonparser', 'sanitizer', 'testsingleton'])
+                        );
+                        expect(Snowboard.testSingleton).toEqual(expect.any(Function));
+
+                        const instance = Snowboard.testSingleton();
 
                         // Check plugin injected methods
-                        expect(instance.snowboard).toBe(Snowboard);
+                        expect(instance.snowboard).toBeDefined();
+                        expect(instance.snowboard.getPlugin).toEqual(expect.any(Function));
+                        expect(() => {
+                            const method = instance.snowboard.initialise;
+                        }).toThrow('cannot use');
                         expect(instance.destructor).toEqual(expect.any(Function));
 
                         // Check plugin method
@@ -138,20 +194,20 @@ describe('Snowboard framework', function () {
                         expect(instance.testMethod()).toEqual('Tested');
 
                         // Check multiple instances  (these should all be the same as this instance is a singleton)
-                        const instanceOne = Snowboard.test();
+                        const instanceOne = Snowboard.testSingleton();
                         instanceOne.changed = true;
-                        const instanceTwo = Snowboard.test();
+                        const instanceTwo = Snowboard.testSingleton();
                         expect(instanceOne).toEqual(instanceTwo);
-                        const factory = Snowboard.getPlugin('test');
+                        const factory = Snowboard.getPlugin('testSingleton');
                         expect(factory.getInstances()).toEqual([instance]);
 
                         // Remove plugin
-                        Snowboard.removePlugin('test');
-                        expect(Snowboard.hasPlugin('test')).toEqual(false);
+                        Snowboard.removePlugin('testSingleton');
+                        expect(Snowboard.hasPlugin('testSingleton')).toEqual(false);
                         expect(dom.window.Snowboard.getPluginNames()).toEqual(
                             expect.arrayContaining([ 'jsonparser', 'sanitizer'])
                         );
-                        expect(Snowboard.test).not.toBeDefined();
+                        expect(Snowboard.testSingleton).not.toBeDefined();
 
                         done();
                     } catch (error) {
@@ -380,4 +436,79 @@ describe('Snowboard framework', function () {
                 }
             );
     });
+
+    it('will allow plugins to call other plugin methods', 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/TestDependencyOne.js',
+                'modules/system/tests/js/fixtures/framework/TestSingletonWithDependency.js',
+            ])
+            .render()
+            .then(
+                (dom) => {
+                    // Run assertions
+                    const Snowboard = dom.window.Snowboard;
+                    const instance = Snowboard.testSingleton();
+
+                    expect(instance.dependencyTest()).toEqual('Tested');
+                },
+                (error) => {
+                    throw error;
+                }
+            );
+    });
+
+    it('doesn\'t allow PluginBase or Singleton abstracts to be modified', 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) => {
+                    expect(() => {
+                        dom.window.Snowboard.PluginBase.newMethod = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.PluginBase.destruct = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.PluginBase.prototype.newMethod = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.Singleton.newMethod = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.Singleton.destruct = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+
+                    expect(() => {
+                        dom.window.Snowboard.Singleton.prototype.newMethod = () => {
+                            return true;
+                        };
+                    }).toThrow(TypeError);
+                },
+            );
+    });
 });

modules/system/tests/js/fixtures/framework/TestPlugin.js

@@ -7,5 +7,5 @@
         }
     }
 
-    Snowboard.addPlugin('test', TestPlugin);
+    Snowboard.addPlugin('testPlugin', TestPlugin);
 })(window.Snowboard);

modules/system/tests/js/fixtures/framework/TestSingleton.js

@@ -7,5 +7,5 @@
         }
     }
 
-    Snowboard.addPlugin('test', TestSingleton);
+    Snowboard.addPlugin('testSingleton', TestSingleton);
 })(window.Snowboard);

modules/system/tests/js/fixtures/framework/TestSingletonWithDependency.js

@@ -19,6 +19,10 @@
         testMethod() {
             return 'Tested';
         }
+
+        dependencyTest() {
+            return this.snowboard.testDependencyOne().testMethod();
+        }
     }
 
     Snowboard.addPlugin('testSingleton', TestSingletonWithDependency);