diff --git a/htdocs/categories/fiche.php b/htdocs/categories/fiche.php index ac5c9a1c978..15ccad5b5d1 100644 --- a/htdocs/categories/fiche.php +++ b/htdocs/categories/fiche.php @@ -38,7 +38,6 @@ $action = GETPOST('action','alpha'); $cancel = GETPOST('cancel','alpha'); $origin = GETPOST('origin','alpha'); $catorigin = GETPOST('catorigin','int'); -$nbcats = (GETPOST('choix') ? GETPOST('choix') : 1); // TODO not use ? $type = GETPOST('type','alpha'); $urlfrom = GETPOST('urlfrom','alpha'); diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 5dfcda5ab17..90167a94b87 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -249,18 +249,20 @@ function GETPOST($paramname,$check='',$method=0) elseif ($method==1) $out = isset($_GET[$paramname])?$_GET[$paramname]:''; elseif ($method==2) $out = isset($_POST[$paramname])?$_POST[$paramname]:''; elseif ($method==3) $out = isset($_POST[$paramname])?$_POST[$paramname]:(isset($_GET[$paramname])?$_GET[$paramname]:''); + else return 'BadParameter'; if (! empty($check)) { + $out=trim($out); // Check if numeric - if ($check == 'int' && ! preg_match('/^[-\.,0-9]+$/i',trim($out))) $out=''; + if ($check == 'int' && ! preg_match('/^[-\.,0-9]+$/i',$out)) $out=''; // Check if alpha - //if ($check == 'alpha' && ! preg_match('/^[ =:@#\/\\\(\)\-\._a-z0-9]+$/i',trim($out))) $out=''; - // '"' is dangerous because param in url can close the href= or src= and add javascript functions. - if ($check == 'alpha') + elseif ($check == 'alpha') { - if (preg_match('/"/',trim($out))) $out=''; - else if (preg_match('/(\.\.\/)+/',trim($out))) $out=''; + // '"' is dangerous because param in url can close the href= or src= and add javascript functions. + // '../' is dangerous because it allows dir transversals + if (preg_match('/"/',$out)) $out=''; + else if (preg_match('/\.\.\//',$out)) $out=''; } } diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index ef231baa464..10d395ff94e 100755 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -144,6 +144,7 @@ class SecurityTest extends PHPUnit_Framework_TestCase $_POST["param1"]="333"; $_GET["param2"]='a/b#e(pr)qq-rr\cc'; $_GET["param3"]='"a/b#e(pr)qq-rr\cc'; // Same than param2 + " + $_GET["param4"]='../dir'; $result=GETPOST('id','int'); // Must return nothing print __METHOD__." result=".$result."\n"; @@ -161,11 +162,15 @@ class SecurityTest extends PHPUnit_Framework_TestCase print __METHOD__." result=".$result."\n"; $this->assertEquals($result,$_GET["param2"]); - $result=GETPOST("param3",'alpha'); // Must return '' as there is a forbidden char + $result=GETPOST("param3",'alpha'); // Must return '' as there is a forbidden char " print __METHOD__." result=".$result."\n"; $this->assertEquals($result,''); - return $result; + $result=GETPOST("param4",'alpha'); // Must return '' as there is a forbidden char ../ + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result,''); + + return $result; } /**