diff --git a/api/src/loader/security.php b/api/src/loader/security.php index 7614b93b77..bc30dccc28 100755 --- a/api/src/loader/security.php +++ b/api/src/loader/security.php @@ -19,8 +19,9 @@ use EGroupware\Api; * @internal * @param array &$var reference of array to check * @param string $name ='' name of the array + * @param boolean $log = true Log the results of checking to the error log */ -function _check_script_tag(&$var,$name='') +function _check_script_tag(&$var,$name='',$log=true) { static $preg=null; //old: '/<\/?[^>]*\b(iframe|script|javascript|on(before)?(abort|blur|change|click|dblclick|error|focus|keydown|keypress|keyup|load|mousedown|mousemove|mouseout|mouseover|mouseup|reset|select|submit|unload))\b[^>]*>/i'; @@ -52,14 +53,17 @@ function _check_script_tag(&$var,$name='') $_REQUEST[$key] = $var[$key] = json_encode($json_data); continue; } - error_log(__FUNCTION__."(,$name) ${name}[$key] = ".$var[$key]); + //error_log(__FUNCTION__."(,$name) ${name}[$key] = ".$var[$key]); $GLOBALS['egw_unset_vars'][$name.'['.$key.']'] = $var[$key]; // attempt to clean the thing $var[$key] = $val = Api\Html\HtmLawed::purify($val); // check if we succeeded, if not drop the var anyway, keep the egw_unset_var in any case if (preg_match($preg,$val)) { - error_log("*** _check_script_tag($name): unset(${name}[$key]) with value $val***"); + if($log) + { + error_log("*** _check_script_tag($name): unset(${name}[$key]) with value $val***"); + } unset($var[$key]); } } @@ -70,105 +74,6 @@ function _check_script_tag(&$var,$name='') } } -/* some _check_script_tag tests, should be commented out by default -if (isset($_SERVER['SCRIPT_FILENAME']) && $_SERVER['SCRIPT_FILENAME'] == __FILE__) // some tests -{ - if (!defined('EGW_INCLUDE_ROOT')) - { - define(EGW_INCLUDE_ROOT, realpath(dirname(__FILE__).'/../..')); - define(EGW_API_INC, realpath(dirname(__FILE__))); - } - - $total = $num_failed = 0; - $patterns = array( - // pattern => true: should fail, false: should not fail - '< script >alert(1)< / script >' => true, - 'blah' => true, - 'Click Me' => true, - // from https://www.acunetix.com/websitesecurity/cross-site-scripting/ - '
".Api\Html::htmlspecialchars($pattern).' '. - (isset($GLOBALS['egw_unset_vars'])?'removed':'passed')."
Could NOT download or parse $url with attack vectors!
Attacks from $url with ".count($vectors)." tests:
".(1+$line).": ".Api\Html::htmlspecialchars($pattern).' '. - (isset($GLOBALS['egw_unset_vars'])?'removed':'passed')."
Tests finished: $num_failed / $total failed
\n"; - foreach(array( - // things unsafe to unserialize - "O:34:\"Horde_Kolab_Server_Decorator_Clean\":2:{s:43:\"\x00Horde_Kolab_Server_Decorator_Clean\x00_server\";" => false, - "O:20:\"Horde_Prefs_Identity\":2:{s:9:\"\x00*\x00_prefs\";O:11:\"Horde_Prefs\":2:{s:8:\"\x00*\x00_opts\";a:1:{s:12:\"sizecallback\";" => false, - "a:2:{i:0;O:12:\"Horde_Config\":1:{s:13:\"\x00*\x00_oldConfig\";s:#{php_injection.length}:\"#{php_injection}\";}i:1;s:13:\"readXMLConfig\";}}" => false, - 'a:6:{i:0;i:0;i:1;d:2;i:2;s:4:"ABCD";i:3;r:3;i:4;O:8:"my_Class":2:{s:1:"a";r:6;s:1:"b";N;};i:5;C:16:"SplObjectStorage":14:{x:i:0;m:a:0:{}}' => false, - serialize(new stdClass()) => false, - serialize(array(new stdClass(), new SplObjectStorage())) => false, - // string content, safe to unserialize - serialize('O:8:"stdClass"') => true, - serialize('C:16:"SplObjectStorage"') => true, - serialize(array('a' => 'O:8:"stdClass"', 'b' => 'C:16:"SplObjectStorage"')) => true, - // false positive: failing our php<7 regular expression, because it has correct delimiter (^|;|{) in front of pattern :-( - serialize('O:8:"stdClass";C:16:"SplObjectStorage"') => true, - ) as $str => $result) - { - if ((bool)($r=php_safe_unserialize($str)) !== $result) - { - if (!$result) - { - if (PHP_VERSION >= 7) - { - if (preg_match_all('/([^ ]+) Object\(/', array2string($r), $matches)) - { - foreach($matches[1] as $class) - { - if (!preg_match('/^__PHP_Incomplete_Class(#\d+)?$/', $class)) - { - echo "FAILED: $str\n"; - continue 2; - } - } - } - echo "passed: ".array2string($str)." = ".array2string($r)."\n"; - } - else - { - echo "FAILED: $str\n"; - } - } - else - { - echo "false positive: $str\n"; - } - } - else - { - echo "passed: $str\n"; - } - //echo "result=".array2string($result).", php_save_unserialize('".htmlspecialchars($str)."') = ".array2string(php_safe_unserialize($str))." --> ".array2string((bool)php_safe_unserialize($str))."\n"; - } -}*/ - /** * Unserialize a json or php serialized array * diff --git a/api/src/loader/test/SecurityTest.php b/api/src/loader/test/SecurityTest.php new file mode 100644 index 0000000000..7f2c95a51a --- /dev/null +++ b/api/src/loader/test/SecurityTest.php @@ -0,0 +1,204 @@ +assertEquals(isset($GLOBALS['egw_unset_vars']), $should_fail); + } + + public function patternProvider() + { + return array( + // pattern, true: should fail, false: should not fail + Array('< script >alert(1)< / script >', true), + Array('blah', true), + Array('Click Me', true), + // from https://www.acunetix.com/websitesecurity/cross-site-scripting/ + Array('', true), + Array('', true), + Array('', true), + Array('', true), + Array('', true), + Array('', true), + Array('', true), + Array('', true), + Array('', true), + Array('', true), + // false positiv tests + Array('If 1 < 2, what does that mean for description, if 2 > 1.', false), + Array('If 1 < 2, what does that mean for a script, if 2 > 1.', false), + Array('Script and Javascript: not evil ;-)', false), + Array('style=background-color', false), + Array('Hugo Sonstwas', false), + Array('', false) + ); + } + + /** + * Test some URLs with bad stuff + * + * @param String $url + * @param Array $vectors + * + * @dataProvider urlProvider + */ + public function testURLs($url, $vectors = FALSE) + { + // no all xss attack vectors from http://ha.ckers.org/xssAttacks.xml are relevant here! (needs interpretation) + if (!$vectors) + { + $this->markAsSkipped("Could not download or parse $url with attack vectors"); + return; + } + foreach($vectors as $line => $pattern) + { + $test = array($pattern); + _check_script_tag($test, 'line '.(1+$line), false); + + $this->assertTrue(isset($GLOBALS['egw_unset_vars']), $line . ': ' . $pattern); + } + } + + public function urlProvider() + { + $urls = array( + // we currently fail 76 of 666 test, thought they seem not to apply to our use case, as we check request data + 'https://gist.github.com/JohannesHoppe/5612274' => file( + 'https://gist.githubusercontent.com/JohannesHoppe/5612274/raw/60016bccbfe894dcd61a6be658a4469e403527de/666_lines_of_XSS_vectors.html'), + // we currently fail 44 of 140 tests, thought they seem not to apply to our use case, as we check request data + 'https://html5sec.org/' => call_user_func(function() { + $payloads = $items = null; + if (!($items_js = file_get_contents('https://html5sec.org/items.js')) || + !preg_match_all("|^\s+'data'\s+:\s+'(.*)',$|m", $items_js, $items, PREG_PATTERN_ORDER) || + !($payload_js = file_get_contents('https://html5sec.org/payloads.js')) || + !preg_match_all("|^\s+'([^']+)'\s+:\s+'(.*)',$|m", $payload_js, $payloads, PREG_PATTERN_ORDER)) + { + return false; + } + $replace = array( + "\\'" => "'", + '\\\\'=> '\\,', + '\r' => "\r", + '\n' => "\n", + ); + foreach($payloads[1] as $n => $from) { + $replace['%'.$from.'%'] = $payloads[2][$n]; + } + return array_map(function($item) use ($replace) { + return strtr($item, $replace); + }, $items[1]); + }), + ); + + $test_data = array(); + + foreach($urls as $url => $vectors) + { + $test_data[] = array( + $url, $vectors + ); + } + return $test_data; + } + + /** + * Test safe unserialization + * + * @param String $str Serialized string to be checked + * @param boolean $result If we expect the string to fail or not + * + * @dataProvider unserializeProvider + */ + public function testUnserialize($str, $result) + { + $r=@php_safe_unserialize($str); + + if((bool)($r) !== $result) + { + if (!$result) + { + if (PHP_VERSION >= 7) + { + if (preg_match_all('/([^ ]+) Object\(/', array2string($r), $matches)) + { + foreach($matches[1] as $class) + { + if (!preg_match('/^__PHP_Incomplete_Class(#\d+)?$/', $class)) + { + $this->fail($str); + } + } + } + } + else + { + $this->fail($str); + } + } + else + { + $this->fail("false positive: $str"); + } + } + } + + /** + * Data set for unserialize test + */ + public function unserializeProvider() + { + return array( + // Serialized string, expected result + // things unsafe to unserialize + Array("O:34:\"Horde_Kolab_Server_Decorator_Clean\":2:{s:43:\"\x00Horde_Kolab_Server_Decorator_Clean\x00_server\";", false), + Array("O:20:\"Horde_Prefs_Identity\":2:{s:9:\"\x00*\x00_prefs\";O:11:\"Horde_Prefs\":2:{s:8:\"\x00*\x00_opts\";a:1:{s:12:\"sizecallback\";", false), + Array("a:2:{i:0;O:12:\"Horde_Config\":1:{s:13:\"\x00*\x00_oldConfig\";s:#{php_injection.length}:\"#{php_injection}\";}i:1;s:13:\"readXMLConfig\";}}", false), + Array('a:6:{i:0;i:0;i:1;d:2;i:2;s:4:"ABCD";i:3;r:3;i:4;O:8:"my_Class":2:{s:1:"a";r:6;s:1:"b";N;};i:5;C:16:"SplObjectStorage":14:{x:i:0;m:a:0:{}}', false), + Array(serialize(new \stdClass()), false), + Array(serialize(array(new \stdClass(), new \SplObjectStorage())), false), + // string content, safe to unserialize + Array(serialize('O:8:"stdClass"'), true), + Array(serialize('C:16:"SplObjectStorage"'), true), + Array(serialize(array('a', 'O:8:"stdClass"', 'b', 'C:16:"SplObjectStorage"')), true), + // false positive: failing our php<7 regular expression, because it has correct delimiter (^|;|{) in front of pattern :-( + Array(serialize('O:8:"stdClass";C:16:"SplObjectStorage"'), true), + ); + } +}