FIX XSS

Dolibarr · May 10, 2017 · 5c33c17 · 5c33c17
1 parent 09f6fe5
commit 5c33c17
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 8 deletions.
diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php
@@ -380,7 +380,7 @@ function GETPOST($paramname, $check='', $method=0, $filter=NULL, $options=NULL)
 	            break;
 	        case 'aZ09':
 	            $out=trim($out);
-	            if (preg_match('/[^a-z0-9_]+/i',$out)) $out='';
+	            if (preg_match('/[^a-z0-9_\-]+/i',$out)) $out='';
 	            break;
 	        case 'array':
 	            if (! is_array($out) || empty($out)) $out=array();
@@ -3104,15 +3104,17 @@ function dol_print_error($db='',$error='',$errors=null)
 		if ($_SERVER['DOCUMENT_ROOT'])  // Mode web
 		{
 			$out.="<b>".$langs->trans("DatabaseTypeManager").":</b> ".$db->type."<br>\n";
-			$out.="<b>".$langs->trans("RequestLastAccessInError").":</b> ".($db->lastqueryerror()?$db->lastqueryerror():$langs->trans("ErrorNoRequestInError"))."<br>\n";
+			$out.="<b>".$langs->trans("RequestLastAccessInError").":</b> ".($db->lastqueryerror()?dol_escape_htmltag($db->lastqueryerror()):$langs->trans("ErrorNoRequestInError"))."<br>\n";
 			$out.="<b>".$langs->trans("ReturnCodeLastAccessInError").":</b> ".($db->lasterrno()?$db->lasterrno():$langs->trans("ErrorNoRequestInError"))."<br>\n";
 			$out.="<b>".$langs->trans("InformationLastAccessInError").":</b> ".($db->lasterror()?$db->lasterror():$langs->trans("ErrorNoRequestInError"))."<br>\n";
 			$out.="<br>\n";
 		}
 		else                            // Mode CLI
 		{
 			$out.='> '.$langs->transnoentities("DatabaseTypeManager").":\n".$db->type."\n";
-			$out.='> '.$langs->transnoentities("RequestLastAccessInError").":\n".($db->lastqueryerror()?$db->lastqueryerror():$langs->trans("ErrorNoRequestInError"))."\n";
+			$out.='> '.$langs->transnoentities("RequestLastAccessInError").":\n".($db->lastqueryerror()?dol_escape_htmltag($db->lastqueryerror()):$langs->trans("ErrorNoRequestInError"))."\n";
+			// To make detection of xss vulnerabilities or sql injection easier with a scanner, replace line with this one:
+			//$out.='> '.$langs->transnoentities("RequestLastAccessInError").":\n".($db->lastqueryerror()?$db->lastqueryerror:$langs->trans("ErrorNoRequestInError"))."\n";
 			$out.='> '.$langs->transnoentities("ReturnCodeLastAccessInError").":\n".($db->lasterrno()?$db->lasterrno():$langs->trans("ErrorNoRequestInError"))."\n";
 			$out.='> '.$langs->transnoentities("InformationLastAccessInError").":\n".($db->lasterror()?$db->lasterror():$langs->trans("ErrorNoRequestInError"))."\n";
 

diff --git a/htdocs/index.php b/htdocs/index.php
@@ -55,10 +55,10 @@
 if (GETPOST('addbox'))	// Add box (when submit is done from a form when ajax disabled)
 {
 	require_once DOL_DOCUMENT_ROOT.'/core/class/infobox.class.php';
-	$zone=GETPOST('areacode');
-	$userid=GETPOST('userid');
-	$boxorder=GETPOST('boxorder');
-	$boxorder.=GETPOST('boxcombo');
+	$zone=GETPOST('areacode', 'aZ09');
+	$userid=GETPOST('userid', 'int');
+	$boxorder=GETPOST('boxorder', 'aZ09');
+	$boxorder.=GETPOST('boxcombo', 'aZ09');
 
 	$result=InfoBox::saveboxorder($db,$zone,$boxorder,$userid);
 	if ($result > 0) setEventMessages($langs->trans("BoxAdded"), null);

diff --git a/htdocs/langs/en_US/agenda.lang b/htdocs/langs/en_US/agenda.lang
@@ -76,6 +76,7 @@ ProposalDeleted=Proposal deleted
 OrderDeleted=Order deleted
 InvoiceDeleted=Invoice deleted
 ##### End agenda events #####
+AgendaModelModule=Document templates for event
 DateActionStart=Start date
 DateActionEnd=End date
 AgendaUrlOptions1=You can also add following parameters to filter output:

diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php
@@ -147,7 +147,9 @@ public function testGETPOST()
 		$_GET["param2"]='a/b#e(pr)qq-rr\cc';
         $_GET["param3"]='"a/b#e(pr)qq-rr\cc';    // Same than param2 + "
         $_GET["param4"]='../dir';
-
+        $_GET["param5"]="a_1-b";
+
+        // Test int
         $result=GETPOST('id','int');              // Must return nothing
         print __METHOD__." result=".$result."\n";
         $this->assertEquals($result,'');
@@ -160,6 +162,7 @@ public function testGETPOST()
         print __METHOD__." result=".$result."\n";
         $this->assertEquals($result,333);
 
+        // Test alpha
         $result=GETPOST("param2",'alpha');
         print __METHOD__." result=".$result."\n";
         $this->assertEquals($result,$_GET["param2"]);
@@ -172,6 +175,27 @@ public function testGETPOST()
         print __METHOD__." result=".$result."\n";
         $this->assertEquals($result,'');
 
+        // Test aZ09
+        $result=GETPOST("param1",'aZ09');  // Must return '' as there is a forbidden char ../
+        print __METHOD__." result=".$result."\n";
+        $this->assertEquals($result,$_GET["param1"]);
+
+        $result=GETPOST("param2",'aZ09');  // Must return '' as there is a forbidden char ../
+        print __METHOD__." result=".$result."\n";
+        $this->assertEquals($result,'');
+
+        $result=GETPOST("param3",'aZ09');  // Must return '' as there is a forbidden char ../
+        print __METHOD__." result=".$result."\n";
+        $this->assertEquals($result,'');
+
+        $result=GETPOST("param4",'aZ09');  // Must return '' as there is a forbidden char ../
+        print __METHOD__." result=".$result."\n";
+        $this->assertEquals($result,'');
+
+        $result=GETPOST("param5",'aZ09');
+        print __METHOD__." result=".$result."\n";
+        $this->assertEquals($result,$_GET["param5"]);
+
         return $result;
     }