3
\$\begingroup\$

I am using EdgeJS in my application. The application is used to run a user-provided javascript program from C#. Because we allow execution of user scripts some sandboxing is required. I am using vm2 for sandboxing. Is it correct to add custom logic by overriding a function exposed by NodeJS library?

Here is the C# program:

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using EdgeJs;
using Newtonsoft.Json;

namespace EdgeJSApplication
{
    class Program
    {
        private const string SWSANDBOXEDSCRIPTIPLISTING = "EdgeJSApplication.NodeScripts.SandboxedScriptIPListing.js";
        private const string SANDBOXEDSCRIPT = "EdgeJSApplication.NodeScripts.SandboxedScript.js";
        private const string SAMPLE_SCRIPT = "EdgeJSApplication.NodeScripts.SampleScript.js";

        static void Main(string[] args)
        {
            try
            {
                //chrome://inspect/#devices
                //Environment.SetEnvironmentVariable("EDGE_NODE_PARAMS", $"--max_old_space_size=2048 --inspect-brk");

                Environment.SetEnvironmentVariable("EDGE_NODE_PARAMS", $"--max_old_space_size=2048");

                var sanboxedScript = GetScript(SWSANDBOXEDSCRIPTIPLISTING);
                var script = GetScript(SAMPLE_SCRIPT);

                var func = Edge.Func(sanboxedScript);

                var executorSettings = new Dictionary<string, object>();
                executorSettings.Add("Tokens", null);

                var funcTask = func(new
                {
                    Script = script,
                    ScriptExecutionContext = new
                    {
                        Tokens = new
                        {
                            OutputFileName = "",
                            IsOutgoingIpWhitelistEnabled = "true",
                        }
                    },
                    AssemblyDirectory = GetAssemblyDirectory()
                });

                dynamic scriptResult = funcTask.Result;

                var json = JsonConvert.SerializeObject(scriptResult.output);

                Console.WriteLine(json);

                Console.Read();
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
            }
        }

        private static string GetScript(string script)
        {
            var assembly = Assembly.GetExecutingAssembly();
            string sanboxedScript;

            using (var stream = assembly.GetManifestResourceStream(script))
            {
                if (stream != null)
                {
                    using (var reader = new StreamReader(stream))
                    {
                        sanboxedScript = reader.ReadToEnd();
                    }
                }
                else
                {
                    throw new ArgumentNullException("Script not found!");
                }
            }

            return sanboxedScript;
        }

        private static string GetAssemblyDirectory()
        {
            string codeBase = typeof(Program).Assembly.CodeBase;
            UriBuilder uri = new UriBuilder(codeBase);
            string path = Uri.UnescapeDataString(uri.Path);
            var assemblyDirectory = Path.GetDirectoryName(path);
            return assemblyDirectory;
        }
    }
}

This program calls a Sandbox script and provides it with an input of user-provided script. Below is the sandbox script and user-provided script

Sandbox script:

        var func = function (context, postresult) {
        const { NodeVM, VM, VMScript } = require('vm2');

        var scriptExecutionContext = context.ScriptExecutionContext;

        const vm = new NodeVM({
            console: 'inherit',
            setTimeout: setTimeout,
            require: {
                external: {
                    modules:['request', 'xpath', 'xmldom', 'xml2js']
                },
                builtin: false,
                context: 'host'
            },
            root: './ ',
            sandbox: {
                context: scriptExecutionContext,    
                callback: callback,
                eval: undefined
            }
        });


        var script = context.Script;
        vm.run(script, context.AssemblyDirectory + '\\edge\\SampleScript.js');

        process.on('uncaughtException',
            (err) => {
                postresult(null,
                    {
                        output: null,
                        previousRunContext: null,
                        scriptLogs: [],
                        success: false,
                        errormessage: "SCRIPTUNCAUGHTEXCEPTION: " + err.message
                    });
            });

        function callback(scope, result) {
            result.success = true;
            postresult(scope, result);
        }
    };
    return func;

User-Provided script:

debugger;
var httpRequest = require('request');
var xPath = require('xpath');
var xmlDOM = require('xmldom');
var parser = require('xml2js');
//var express = require('express'); //external
//var fs = require('fs'); //builtin

var someURI = "https://api.github.com/users";
var someMethod = "GET";
var someHeaders = {
    'User-Agent': 'Node.js'
}
makeRequest(someURI, someMethod, someHeaders);

function makeRequest(httpEndpoint, httpMethod, httpHeaders) {
    httpRequest(
        {
            method: httpMethod,
            uri: httpEndpoint,
            headers: httpHeaders
        },
        function handleResponse(error, response, body) {
            debugger;
            if (response.statusCode == 200) {
                callback(null, {
                    output: JSON.parse(body),
                    previousRunContext: "myContextVariable"
                });
            }
            else {
                throw new Error("The request did not return a 200 (OK) status.\r\nThe returned error was:\r\n" + error);
            }
        }
    );
}

The users are allowed to use only whitelisted external modules in their script. This is controlled from the following code block in Sandbox script.

require: {
            external: {
                modules:['request', 'xpath', 'xmldom', 'xml2js']
            },
            builtin: false,
            context: 'host'
        }

So if in the sample program following lines are uncommented it will return error

//var express = require('express'); //external
//var fs = require('fs'); //builtin

Recently I was asked to add a validation to check if the IPs used in the user-provided script are whitelisted. This is how I implemented it:

    var func = function (context, postresult) {
    debugger;
    const { NodeVM, VM, VMScript } = require('vm2');
    const extend = require('extend');
    const dns = require('dns');
    const url = require('url');
    var whitelistedIps = ['140.82.118.5'];
    var scriptExecutionContext = context.ScriptExecutionContext;

    var prepareRequire = function (){}
    var modifiedRequire = function (module) {
        console.log(module);
        var preparedRequire = prepareRequire(module);
        if (module === 'request') {
            return function request(uri, options, callback) {
                var mergedInput = mergeInput(uri, options, callback);
                var uri = url.parse(mergedInput.uri);
                if (isValidIpAddress(uri.host)) {
                    if (!isAllowedIP(uri.host)) {
                        let message = `Data Request IP (${uri.host}) does not fall within the outgoing IP whitelist range.`;
                        throw new Error(message);
                    }
                    else {
                        return preparedRequire(mergedInput, mergedInput.callback);
                    }
                }
                else {
                    dns.lookup(uri.host, function (err, result) {
                        if (result) {
                            if (!isAllowedIP(result)) {
                                let message = `Data Request URI (${uri.host}) whose IP is (${result}) does not fall within the outgoing IP whitelist range.`;
                                throw new Error(message);
                            }
                            else {
                                return preparedRequire(mergedInput, mergedInput.callback);
                            }
                        }
                        else {
                            throw err;
                        }
                    });
                }
            }
        }
        return preparedRequire;
    }

    const vm = new NodeVM({
        console: 'inherit',
        setTimeout: setTimeout,
        require: {
            external: {
                modules:['request', 'xpath', 'xmldom', 'xml2js']
            },
            builtin: false,
            context: 'host'
        },
        root: './ ',
        sandbox: {
            context: scriptExecutionContext,    
            callback: callback,
            eval: undefined,
            modifiedRequire: modifiedRequire
        }
    });

    prepareRequire = vm._prepareRequire(context.AssemblyDirectory+ '\\edge');

    var script = `require = modifiedRequire;\n` + context.Script;
    vm.run(script, context.AssemblyDirectory + '\\edge\\SampleScript.js');

    process.on('uncaughtException',
        (err) => {
            postresult(null,
                {
                    output: null,
                    previousRunContext: null,
                    scriptLogs: [],
                    success: false,
                    errormessage: "SCRIPTUNCAUGHTEXCEPTION: " + err.message
                });
        });

    function callback(scope, result) {
        result.success = true;
        postresult(scope, result);
    }

    function isAllowedIP(ipAddress) {
        let ipAddressInt = toInt(ipAddress);
        for (let index = 0; index < whitelistedIps.length; index++) {
            var val = whitelistedIps[index];
            if (val.indexOf('-') !== -1) {
                let ipRange = val.split('-');
                let minIp = toInt(ipRange[0]);
                let maxIp = toInt(ipRange[1]);
                if (ipAddressInt >= minIp && ipAddressInt <= maxIp) {
                    return true;
                }
            }
            else {
                let ip = toInt(val);
                if (ip === ipAddressInt) {
                    return true;
                }
            }
        }

        return false;
    }

    function mergeInput(uri, options, callback) {
        if (typeof options === 'function') {
            callback = options
        }

        var mergedInput = {}
        if (options !== null && typeof options === 'object') {
            extend(mergedInput, options, { uri: uri })
        }
        else if (typeof uri === 'string') {
            extend(mergedInput, { uri: uri })
        }
        else {
            extend(mergedInput, uri)
        }

        mergedInput.callback = callback || mergedInput.callback
        return mergedInput
    }

    function toInt(ip) {
        return ip.split('.').map((octet, index, array) => {
            return parseInt(octet) * Math.pow(256, (array.length - index - 1));
        }).reduce((prev, curr) => {
            return prev + curr;
        });
    }

    function isValidIpAddress(value) {
        const regexIP = /\b((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\b/;
        return regexIP.test(value);
    }
};
return func;

Basically I am overriding the _prepareRequire function and added logic to check IP validity if the module is of type request. If the module is any other module, then I execute the _prepareRequire function provided by vm2 library as is. This is working as expected. I would like to know if there is any issue with this approach?

The complete program can be found here

\$\endgroup\$
  • \$\begingroup\$ Why was this question downvoted? What's wrong? \$\endgroup\$ – Aashish Upadhyay Oct 2 at 19:08
  • 1
    \$\begingroup\$ I'll take a guess since i'm not the one who downvoted : You tagged your question with c#, I don't think this is accurate. And you title is probably too vague. You should try to edit it to make it clearer. Apart from those two points, I'm not sure I see why your question would've been downvoted. Is the code supplied sufficient to understand the scope of the problem? \$\endgroup\$ – IEatBagels Oct 2 at 19:55
  • \$\begingroup\$ Thanks, I will make the necessary edits. The code above will explain the scope. However, that in itself is not enough to run the application, I have provided a link to github repo where entire working code is present. \$\endgroup\$ – Aashish Upadhyay Oct 2 at 20:00
  • 2
    \$\begingroup\$ Maybe this is the reason, then. The thing with CodeReview is that you must have all the code necessary to understand the context (so, is every variable/function you use in the code above defined in the said code or would someone need to follow the GitHub link?). It's hard to make it right, but it's worth it. Try to make sure someone can understand the whole code without having to follow external links. \$\endgroup\$ – IEatBagels Oct 2 at 20:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.