JavaScript escaping

2010-02-03
2013-05-30
  • Micah Stetson
    Micah Stetson
    2010-02-03

    In skimming the new code from the last two weeks, I've come across things like this:

    $('#foo').html('<?php echo T('Foo') ?>');
    

    and this

    echo "$('#bar').val('$_REQUEST[bar]');";
    

    The first could result in weird translation errors and the second is an XSS vulnerability.  The answer here is to add a function to escape values to be placed in JavaScript string literals.  I've just added the JS() function to shared/common.php to do just that.  Output from JS() will not contain characters that can confuse HTML, so there's no need to do H(JS($foo)).  Here are the rewritten code fragments:

    $('#foo').html('<?php echo JS(T('Foo')) ?>');
    
    echo "$('#bar').val('".JS($_REQUEST['bar'])."');";
    

    I've added comments about this to the development wiki.

    If you've been guilty of this sort of escaping sin, please fix it up.

    Micah

     
  • Micah Stetson
    Micah Stetson
    2010-02-03

    Hmmn.  The bottom of that message seems messed up.  Here's what it should have looked like:

    I've added comments about this to the development wiki.

    If you've been guilty of this sort of escaping sin, please fix it up.

    Micah

     
  • Fred LaPlante
    Fred LaPlante
    2010-02-04

    I am sure I am the source of most of what you are pointing out. I see what you are trying to do, but honestly don't see the problems you are trying to avoid. I haven't noticed any translation oddities and 'XSS vulnerability' is over my head completely. 

    In my own world, all this code is used inside a controlled environment - local LAN - so haven't been very concerned about external security. Perhaps that's blissful ignorance, plain good luck, or something.

    Fred

     
  • Micah Stetson
    Micah Stetson
    2010-02-07

    An English-speaking home user will never run into either of these problems.

    The first problem will cause frustration for translators down the road.  Imagine that the proper translation of 'Foo' in some language includes an apostrophe - say it's "B'ar".  Then the JS code from OpenBiblio becomes:

    $('#foo').html('B'ar');
    

    A syntax error.  The translators may not know JavaScript, and even if they do, how easy is this going to be to catch from what they'll see in the browser?  Better to avoid the problem.

    XSS is more serious.  It stands for cross-site scripting - the ability for a malicious link to insert JavaScript into the page.  Admittedly, it's not a concern for a home user, but it's not something OpenBiblio can ignore.  Imagine this: an institution that's a target (e.g. one that handles a lot of money, has a large database of personal information, whatever) has a small internal library that runs OpenBiblio.  An XSS vulnerability could be exploited to run arbitrary JavaScript with the permissions and look of a trusted internal company web site.  There are a lot of ways an attacker can go with this, and it can be a key component of a larger attack.  More info is available here among other places.  It's possible to avoid this, and we need to do so.

    Micah

     
  • Fred LaPlante
    Fred LaPlante
    2010-02-08

    Good points, and never occurred to me at all. Guess I'll have to find a way to get your JS() function into my stuff.

    Incidently the current split still useful in your opinion? I'm not sure its gaining anything unless you want to protect your work. Trying to take advantage of what you are doing seems awkward at best.

    Fred

     
  • Micah Stetson
    Micah Stetson
    2010-02-11

    There are two kinds of split I know of, and I think they have both outlived their usefulness.  The first is the difference between mstetson/obiblio-10-wip and flaplante/obiblio-10-wip.  You and Luuk are doing most of the work right now, and it's silly not to just let you commit to my repo.  I've just given both of you permission to do so; you can push directly there if you like.  The other split is between the "your" code and "my" code.  We want one body of OpenBiblio code.  So when you change or improve something, do it boldly.  Do what you think is right, and don't worry about leaving certain files or lines alone.  If I don't like what you're doing, I'll tell you, and hg gives us an undo button.

    Micah

     
  • Fred LaPlante
    Fred LaPlante
    2010-02-11

    OK, I think you are correct. Time to remerge these things.  Do you want to pull the current stuff from my change set into yours. and then I will abandon mine completely.  If Luuk want to keep it for his multi-site work thats fine with me.

    Let me know when it is OK to start commiting to your repository.

    Fred