Skip to content

Commit

Permalink
Filter input for its use in XPath expressions
Browse files Browse the repository at this point in the history
In order to avoid XPath injection, user input must be filtered before it ends up in the query. Unfortunately, there's no way to do this with a standard method in PHP, so we need our own filtering function. Current best practice recommends using white lists instead of black lists to allow only a subset of characters. In this case, we allow only letters, numericals, spaces, dashes and underscores.

This fixes a bug also inside a loop, where $identifier is used instead of $idKey (the element in the current loop iteration).
  • Loading branch information
jaimeperez committed Feb 15, 2018
1 parent 222303d commit 6490326
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
32 changes: 32 additions & 0 deletions src/Utils/XPath.php
@@ -0,0 +1,32 @@
<?php

namespace RobRichards\XMLSecLibs\Utils;

class XPath
{
const ALPHANUMERIC = 0;
const NUMERIC = 1;
const LETTERS = 2;
const EXTENDED_ALPHANUMERIC = 3;

private static $regex = [
self::ALPHANUMERIC => '#[^\w\d]#',
self::NUMERIC => '#[^\d]#',
self::LETTERS => '#[^\w]#',
self::EXTENDED_ALPHANUMERIC => '#[^\w\d\s-_]#'
];


/**
* Filter a string for save inclusion in an XPath query.
*
* @param string $input The query parameter to filter.
* @param int $allow The character set that we should allow.
*
* @return string The input filtered with only allowed characters.
*/
public static function filter($input, $allow = self::EXTENDED_ALPHANUMERIC)
{
return preg_replace(self::$regex[$allow], '', $input);
}
}
3 changes: 2 additions & 1 deletion src/XMLSecEnc.php
Expand Up @@ -5,6 +5,7 @@
use DOMNode;
use DOMXPath;
use Exception;
use RobRichards\XMLSecLibs\Utils\XPath as XPath;

/**
* xmlseclibs.php
Expand Down Expand Up @@ -470,7 +471,7 @@ public static function staticLocateKeyInfo($objBaseKey=null, $node=null)
}
$id = substr($uri, 1);

$query = "//xmlsecenc:EncryptedKey[@Id='$id']";
$query = "//xmlsecenc:EncryptedKey[@Id='".XPath::filter($id)."']";
$keyElement = $xpath->query($query)->item(0);
if (!$keyElement) {
throw new Exception("Unable to locate EncryptedKey with @Id='$id'.");
Expand Down
5 changes: 3 additions & 2 deletions src/XMLSecurityDSig.php
Expand Up @@ -6,6 +6,7 @@
use DOMNode;
use DOMXPath;
use Exception;
use RobRichards\XMLSecLibs\Utils\XPath as XPath;

/**
* xmlseclibs.php
Expand Down Expand Up @@ -489,10 +490,10 @@ public function processRefNode($refNode)
$xPath->registerNamespace($nspf, $ns);
}
}
$iDlist = '@Id="'.$identifier.'"';
$iDlist = '@Id="'.XPath::filter($identifier).'"';
if (is_array($this->idKeys)) {
foreach ($this->idKeys AS $idKey) {
$iDlist .= " or @$idKey='$identifier'";
$iDlist .= " or @$idKey='".XPATH::filter($idKey)."'";
}
}
$query = '//*['.$iDlist.']';
Expand Down

0 comments on commit 6490326

Please sign in to comment.