Is using new Function considered a security risk?

Question

I have a function to help me create ad hoc objects and save me time typing.

Additional EDIT: to clarify this function will only ever sit inside an anon function.

(function(){ // clarification of the functions location
var objectPump = function (props, defaults){
    var str;
    if(typeof defaults === "string"){
        defaults = defaults.split(",");
    }else    
    if(typeof defaults === "undefined" ||  !defaults.isArray){        
        defaults =[];
    }
    if(props !== undefined){
        if(typeof props === "string"){
           props = props.split(",");
        }
    }else{
        throw new TypeError("No properties defined for objectPump.");
    }
    // create function body
    str = "var obj={};";
    props.each( function(p,i) {  
        str += "obj." + p + "=";
        if (typeof defaults[i] === "string") {
            str += p + "===undefined?" + '"' + defaults[i] + '":';
        } else
        if (typeof defaults[i] === "number") {
            str += p + "===undefined?" + defaults[i] + ":";
        }
        str += p + ";";  
    });
    str += "return obj;";
    str = "return new Function('" + props.join("','") + "','" + str + "')";
    // Uses new Function to create the new function
    return  (new Function(str))();  // Is this dangerous???        
}
})();  // wrapped in an anon function

Which lets me create objects without having to name all the properties and code in defaults.

Edit: Use of the above function.

var car = objectPump("colour,year,type", // objects property names
                     "white,2015,All Wheel Drive"); // object defaults
// or as arrays
var car = objectPump(["colour","year","type"], // objects property names
                     ["white",2015,"All Wheel Drive"]); // object defaults
var cars = [
    car("red",2011), // missing property defaults to All Wheel Drive
    car("blue",2015,"bike"),
];
var aCar =   car("blue",2015,"bike");
// same as
var aCar = {
    colour:"blue",
    year:2015,
    type:"bike"
};  // but saves me having to type out the property names for each new object

To me it looks very similar to using eval and a place a third party harker could get some malicious code in. So far it has been very handy and I am tempted to use new Function for other tasks.

Should I use new Function() to generate code or is it considered bad and/or dangerous for public code.

Could you show how you’d use this utility, please? `new Function` probably isn’t necessary. (And it’s bad practice, yes, but it doesn’t look like there are any vulnerabilities right now… but that’s also hard to say without seeing it in context.) — Ry-, Oct 28 '15 at 08:44
How you're implementing that is a bit convoluted. Doing it the "normal" way would make it cleaner and you wouldn't need eval or `new Function`. — JJJ, Oct 28 '15 at 09:20
If it is wrapped in an anonymous function like that, how do you expose functionality? You set the variable `objectPump` within the anonymous function, but it cannot be reached from outside. — Justus Romijn, Oct 28 '15 at 09:27
Doing it the normal way means I have to type in the property names every time I create an object. I then started writing functions to create the objects, but as the code was so similar just the names and defaults where different I thought why not automate the function writing as well, Then why not have it run live. — Blindman67, Oct 28 '15 at 09:29
@Blindman67 I mean you could make a factory and have the exact same functionality. Try this: http://jsfiddle.net/kk0rzqso/ — JJJ, Oct 28 '15 at 09:30
@Juhana Excellent, I did not think of that. But the question is about the use of `new Function()` the code was just an example. — Blindman67, Oct 28 '15 at 09:34

score 1 · Answer 1 · edited May 23 '17 at 12:14

1

var car = objectPump("colour,script", // objects property names
        "white,\" + alert(\"test\") + \""); // object defaults

console.log(new car('blue, but the nice one')); // throws alert

Do you mean like this dangerous?

To be honest, I don't really like objectPump function. There are other viable options you have:

Use TypeScript and its default values for functions (http://www.typescriptlang.org/Handbook#functions-optional-and-default-parameters)

Use typeof for defining default values even though it's more typing:

function foo(a, b)
{
    a = typeof a !== 'undefined' ? a : 42;
    b = typeof b !== 'undefined' ? b : 'default_b';
    ... 
}

(https://stackoverflow.com/a/894877/99256)

EDIT: The function objectPump does not give your attacker any advantage. 1) If your attacker can modify your JS file, then she will use eval straight away and she does not need any objectPump. 2) If you sanitize all input from your users, there is no problem here.

My primary concern here is that you will eventually shoot yourself in the foot, rather than an attacker will.

edited May 23 '17 at 12:14

Community

1
1

answered Oct 28 '15 at 09:11

MartyIX

27,828
29
136
207

But if I have `objectPump` inside `(function(){ var objectPump= function(){...}; })()` how do they get access to the function to add the bad code. This is what I don't understand. – Blindman67 Oct 28 '15 at 09:16
1

The function `objectPump` does not give your attacker any advantage. 1) If your attacker can modify your JS file, then she will use `eval` straight away and she does not need any `objectPump`. 2) If you sanitize all input from your users, there is no problem here. My primary concern here is that you will eventually shoot yourself in the foot, rather than an attacker will. – MartyIX Oct 28 '15 at 09:22
so what you are saying is that using `new Function` is safe as long as I keep it out of public scope. – Blindman67 Oct 28 '15 at 09:41
Private scope is another layer of "security". You just have to prevent this case: `objectPump('some, parameters', 'THIS INPUT HAS TO BE SANITIZED IF IT IS GIVEN BY A USER');` and you are ok. – MartyIX Oct 28 '15 at 09:49
the user will never get access to the function. Nor do I understand why use `typeof a !== "undefined"` as the variables are defined as arguments, how could `a !== undefined` fail??? – Blindman67 Oct 28 '15 at 10:07
if you call `foo(1)` then `a = 1` and `b = 'default_b'`. if you call `foo()` then `a = 42` and `b = 'default_b'`. – MartyIX Oct 28 '15 at 10:15
@Blindman67: `a !== undefined` can’t fail; don’t use `typeof`. – Ry- Oct 28 '15 at 15:23

Is using new Function considered a security risk?

1 Answers1