[VUFIND-54] cross site scripting vulnerability (XSS) Created: 15/May/09  Updated: 10/Nov/09  Resolved: 10/Nov/09

Status: Closed
Project: VuFind®
Components: User Interface
Affects versions: 1.0RC1
Fix versions: 1.0RC2

Type: Bug Priority: Major
Reporter: Till Kinstler Assignee: Demian Katz
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified
Environment: PHP 5.2.6


 Description   
RC1 and recent SVN checkouts of VuFind don't filter GET/POST (user) input for cross site scripting attacks.
To reproduce: Do a search eg. on <script language='javascript'>alert(document.cookie)</script> in recent VuFind installations, examples:
http://yufind.library.yale.edu/yufind/Search/Home?lookfor=%3Cscript+language%3D%27javascript%27%3Ealert(document.cookie)%3C%2Fscript%3E&submit=Find
or http://discovery.library.colostate.edu/Search/Home?lookfor=%3Cscript+language%3D%27javascript%27%3Ealert(document.cookie)%3C%2Fscript%3E&type=all&submit=Search

 Comments   
Comment by Till Kinstler [ 15/May/09 ]
Possibly a quick fix, BUT NOT INTENSLEY TESTED, MAYBE INCOMPLETE, MAYBE WITH SIDEEFFECTS:
Add the following to web/index.php before anything else happens with $_GET and $_POST:

{code}
foreach ($_GET as &$get) {
  if(is_array($get)) {
    foreach ($get as &$gets) {
      $gets = trim(strip_tags($gets));
    }
  }
  else {
    $get = trim(strip_tags($get));
  }
}

foreach ($_POST as &$get) {
  if(is_array($get)) {
    foreach ($get as &$gets) {
      $gets = trim(strip_tags($gets));
    }
  }
  else {
    $get = trim(strip_tags($get));
  }
}
{code}

Please test and/or suggest more appropriate approaches.
Comment by Till Kinstler [ 18/May/09 ]
$_COOKIE should be handled as untrusted user input as well. So iterate on all $_COOKIEs doing a strip_tags() as well?

Better ideas to adress this issue?
Comment by Till Kinstler [ 08/Jul/09 ]
I now changed that to a recursive function:
Add

function antiXSS($var) {
  if(isset($var)) {
    foreach($var as &$VAR) {
     if(is_array($VAR)) {
       antiXSS($VAR);
      }
      else {
       $VAR = trim(strip_tags($VAR));
      }
    }
  }
}

to index.php.
Then call this function giving it the Arrays to check by reference before doing anything else with user submitted values in index.php, eg before // Start interface:

antiXSS(&$_GET);
antiXSS(&$_POST);
antiXSS(&$_COOKIES);

That's included in the Patch in VUFIND-60
Comment by Demian Katz [ 07/Oct/09 ]
I'm not convinced that there's a one-size-fits-all filtering solution that can make all user input safe -- there will always be edge cases where user data will get mangled. I feel more comfortable tackling this from the other direction and making sure that data output in the templates is always properly escaped. I realize that it's very hard to catch every possible security hole by doing things this way, but it's less likely to cause unanticipated side effects, and there's no harm in it since it can still complement a lower-level solution like Till's proposed quick fix (which might help if you're paranoid, but which I would not advocate incorporating in the trunk code).

I eventually hope to do a thorough review of all templates to identify and fix XSS problems. In the meantime, I have been patching problems whenever I notice them, and I try to inspect all new SVN commits for potential issues. Additionally, injecting Javascript should be part of standard testing procedure whenever we introduce features that take user input to prevent new problems from springing up.
Comment by Demian Katz [ 10/Nov/09 ]
As of r1802, I have reviewed all templates for encoding/escaping problems. This should solve all known XSS vulnerabilities as of this writing, so I am closing the ticket.

Obviously, XSS problems are easily reintroduced, so we need to keep this in mind in the future. Feel free to reopen this (or open a new ticket) if you find new problems or if you have thoughts on more thorough input-side cleaning (since I've really only addressed the output side of the equation for the moment).
Generated at Sun Apr 28 08:13:27 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100251-rev:4690f9fa025ccb713885a7f8212eefdeb0c508be.