CODEWITHSUNDEEP
javascriptsecurity
nico
nico

Reputation: 51640

Dynamic evaluation of functions in JS (is this safe?)

I have a website in which all the pages are processed through an index.php that includes different PHP files depending on the requested URL (this is done through mod_rewrite).

I'm using the following method to execute specific functions at page load:

index.php

<script type="text/javascript">
readyFns = Array();
</script>

<?php
// Do some stuff here, and pull the name of the PHP page to include from the DB
include $pageToInclude
?>

<script type="text/javascript">

    commonFunctionToApplyToAllThePages();
    otherCommonFunction();

    // page-specific functions
    for (i=0; i<readyFns.length; i++)
    {
    if (typeof(window[readyFns[i]]) == "function")
        window[readyFns[i]]();
    }    
</script>

includedPage.php

<?php
// Generate page
?>
<script type="text/javascript">
readyFns.push("someFunction");
readyFns.push("someOtherFunction");
</script>

I quite like this approach because I just have to set readyFns at the end of this page and everything else will be nicely controlled by index.php. My questions is: is this safe? Could it be sensitive to someone generating a link that arbitrarily sets readyFns to point to some malicious code and then links to my site? How would I prevent that?

thanks nico

Upvotes: 0

Views: 310

Answers (1)

bcherry
bcherry

Reputation: 7238

This is interesting. In principle, it's probably ok, but you're right to be a little concerned. This is just compiling a list of keys to lookup as functions on an object, and execute, so it's not really a security problem in that respect. But, you are essentially providing access to all globals like that. You'd probably be better off making a global object besides window to store your functions on, like so:

var funcs = {};
funcs.someFunction = function() {/*blah*/};
funcs.someOther = function() {/*blah*/};

and then your readyFuncs thing would loop over funcs instead of window. I don't think there'd be anything to worry about past that.

Of course, there are other things with your approach that could be improved, but I think it's ok as-is if it works for you.

Upvotes: 1

Related Questions