1

This might be a bit of a specific problem (research project), but I'm looking for a good way to make this program significantly "safer" than it currently is.

The goal is to accept arbitrary user code (after it has passed unit testing and a manual inspection) and incorporate it into a running program without the need to reload the application.

The specific example is that it is a drawing application that continuously executes and I would like students/other users to be able to commit a script (that follows a specific format/guideline).

My current implementation is to require that the user specify that their classname match their filename (e.g., myclass.js has a const myclass = class { ... } block inside). Then when I see that a new file exists within my server (a Flask application that checks the contents of a particular directory), the JavaScript application will load that particular file.

At present I have a promise in place to ensure the script is loaded, but the general method I'm doing is this to bring it into memory:

function loadNewScript(scriptName) {
  let s = document.createElement("script");
  s.setAttribute("type", "text/javascript");
  s.setAttribute("src", `/static/techniques/${scriptName}`);
  let nodes = document.getElementsByTagName("*");
  let node = nodes[nodes.length - 1].parentNode;
  node.appendChild(s);
}

However, the security issue I'm anticipating (other than expecting arbitrary code I suppose) is that I'm currently using eval to bring the class into memory:

// technique comes in as 'myclass.js'
function loadObject(technique) {
    try {
        let obj = eval(technique.split(".")[0]);
        let _activeObj = new obj();
        if (typeof _activeObj != "undefined") {
            return _activeObj;
        }
        return null;
    } 
    catch (e) { // will check for syntax errors, but this usually trips when the script isn't loaded yet
        return null;
    }
}

At present this works, however I'm a bit concerned about the use of eval here. I've seen posts regarding using the window or this namespace, but from my understanding you can't add a dynamically-loaded script into the global namespace? At any rate, using window['myclass'] or this['myclass'] isn't working after it is loaded. (How to execute a JavaScript function when I have its name as a string)

erik
  • 3,810
  • 6
  • 32
  • 63
  • 1
    Regarding `loadNewScript`; you're using a slow method to append the script to the last element. I would just use `document.body.appendChild(s);` (which would append the script to the end of the ``) as opposed to loading every single element in the DOM into a `NodeList`. If you must append it to the last element, you could at least use `document.querySelector('body > *:last-child')` to make it less memory-intensive. – Heretic Monkey May 04 '23 at 17:01
  • 1
    As far as the question goes, you may want to consider using [ECMAScript Modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules) as these have a built-in method for dynamic loading ([`import()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import)) which may work better than anything home-grown. – Heretic Monkey May 04 '23 at 17:04
  • Thanks @HereticMonkey! I'll probably be wanting to eke out better performance so that's good to know. I was thinking it needed to be loaded into the head for some reason but you're right. – erik May 04 '23 at 17:20

1 Answers1

1

No point in avoiding eval for security, really, if you already load "arbitrary" (reviewed) code into dynamic <script>s. The problem why window['myclass'] does not work is because const does declare a global variable but create a property of the global object. You'd need to change the scripts to use var myclass = class { ... }; instead.

However, I would recommend you change your implementation to require files that follow the ES6 module format:

export default class myclass { ... }

and then you can load these using

function loadNewScript(scriptName) {
    return import(`/static/techniques/${scriptName}`);
}
async function loadObject(technique) {
    const module = await loadNewScript(technique);
    const obj = module.default;
    return new obj();
}

If you need the loadObject method to be synchronous (instead of returning a promise), you can use an object or Map as a class registry:

const registry = {};
async function loadNewScript(scriptName) {
    const module = await import(`/static/techniques/${scriptName}`);
    registry[scriptName] = module.default;
}
function loadObject(technique) {
    const obj = registry[technique];
    return obj ? new obj() : null;
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • That's a fair point and I like this approach better than what I have, thanks! – erik May 04 '23 at 17:19
  • This might be a silly question since I don't have as much experience with the ES6 style (I like the approach better to be honest), but what would be the best way to resolve the promise? I get all the techniques loaded in and see that the objects are fulfilled (and I see the object within `[[PromiseResult]]`, but I'm not certain how to access it (or check when it is available in my forever loop. – erik May 04 '23 at 19:17
  • You need to [wait for the promise](https://stackoverflow.com/q/29516390/1048572), just like previously you needed to wait for `loadNewScript` (not sure how you did that, the code in your question has no indication of it). If you don't wait, the class might or might not be available yet (see the second snippet in my answer for that, it just returns `null` when the class was not yet put in the registry) – Bergi May 04 '23 at 19:35
  • Makes sense. Previously I had a Promise that included a call to `setTimeout`, where the script was added to the nodetree within the timeout function. I think I would just need to rectify the syntax since yours is much cleaner than mine :). – erik May 04 '23 at 19:38
  • FWIW, I went with the synchronous call and just checked to see if the value was null or not within the `registry` class, thank you so much! – erik May 04 '23 at 19:39
  • 1
    @erik Ah instead of a timeout you should have used `new Promise((resolve, reject) => { let s = document.createElement("script"); …; s.onload = resolve; s.onerror = reject; …; node.appenChild(s); })` – Bergi May 04 '23 at 19:40