From 819656bd89726bd7974748f88b113dc1d5b4b905 Mon Sep 17 00:00:00 2001
From: Laurent Destailleur <eldy@destailleur.fr>
Date: Sun, 18 Jun 2017 21:27:49 +0200
Subject: [PATCH] Fix security: Check _SERVER["QUERY_STRING"] is escaped.

---
 htdocs/accountancy/admin/accountmodel.php |  2 +-
 htdocs/main.inc.php                       |  2 +-
 test/phpunit/CodingPhpTest.php            | 19 ++++++++++++++++++-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/htdocs/accountancy/admin/accountmodel.php b/htdocs/accountancy/admin/accountmodel.php
index 80dcd2b1807..736338d1b88 100644
--- a/htdocs/accountancy/admin/accountmodel.php
+++ b/htdocs/accountancy/admin/accountmodel.php
@@ -599,7 +599,7 @@ if ($id)
 
     $fieldlist=explode(',',$tabfield[$id]);
 
-    print '<form action="'.dol_escape_htmltag($_SERVER['PHP_SELF']).'?id='.$id.'" method="POST">';
+    print '<form action="'.$_SERVER['PHP_SELF'].'?id='.$id.'" method="POST">';
     print '<input type="hidden" name="token" value="'.$_SESSION['newtoken'].'">';
 
     print '<table class="noborder" width="100%">';
diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php
index 407a0dbb1d0..022aa4d5e44 100644
--- a/htdocs/main.inc.php
+++ b/htdocs/main.inc.php
@@ -99,7 +99,7 @@ function test_sql_and_script_inject($val, $type)
     $sql_inj += preg_match('/base[\s]+href/si', $val);
     $sql_inj += preg_match('/<.*onmouse/si', $val);       // onmousexxx can be set on img or any html tag like <img title='...' onmouseover=alert(1)>
     $sql_inj += preg_match('/onerror\s*=/i', $val);       // onerror can be set on img or any html tag like <img title='...' onerror = alert(1)>
-//    $sql_inj += preg_match('/onfocus\s*=/i', $val);       // onfocus can be set on input text html tag like <input type='text' value='...' onfocus = alert(1)>
+    $sql_inj += preg_match('/onfocus\s*=/i', $val);       // onfocus can be set on input text html tag like <input type='text' value='...' onfocus = alert(1)>
     if ($type == 1)
     {
         $sql_inj += preg_match('/javascript:/i', $val);
diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php
index 581d5cdda89..5df31da812c 100644
--- a/test/phpunit/CodingPhpTest.php
+++ b/test/phpunit/CodingPhpTest.php
@@ -172,9 +172,25 @@ class CodingPhpTest extends PHPUnit_Framework_TestCase
             $this->assertTrue($ok, 'Found non escaped string in building of a sql request '.$file['fullname'].' ('.$val[0].'). Bad.');
             //exit;
 
+            // Test that output of $_SERVER\[\'QUERY_STRING\'\] is escaped.
             $ok=true;
             $matches=array();
             // Check string   ='".$this->xxx   with xxx that is not 'escape'. It means we forget a db->escape when forging sql request.
+            preg_match_all('/(...................)\$_SERVER\[\'QUERY_STRING\'\]/', $filecontent, $matches, PREG_SET_ORDER);
+            foreach($matches as $key => $val)
+            {
+                if ($val[1] != 'dol_escape_htmltag(')
+                {
+                    $ok=false;
+                    break;
+                }
+            }
+            $this->assertTrue($ok, 'Found a $_SERVER[\'QUERY_STRING\'] without dol_escape_htmltag around in file '.$file['fullname'].' ('.$val[1].'$_SERVER[\'QUERY_STRING\']). Bad.');
+
+            // Test that output of $_SERVER\[\'PHP_SELF\'\] is escaped (not done for the moment, did not found a way to forge value of $_SERVER['PHP_SELF'] by extern access).
+            /*$ok=true;
+            $matches=array();
+            // Check string   ='".$this->xxx   with xxx that is not 'escape'. It means we forget a db->escape when forging sql request.
             preg_match_all('/(...................)\$_SERVER\[\'PHP_SELF\'\]/', $filecontent, $matches, PREG_SET_ORDER);
             foreach($matches as $key => $val)
             {
@@ -184,7 +200,8 @@ class CodingPhpTest extends PHPUnit_Framework_TestCase
                     break;
                 }
             }
-            $this->assertTrue($ok, 'Found a $_SERVER[\'QUERY_STRING\'] without dol_escape_htmltag around in file '.$file['fullname'].' ('.$val[1].'$_SERVER[\'QUERY_STRING\']). Bad.');
+            $this->assertTrue($ok, 'Found a $_SERVER[\'PHP_SELF\'] without dol_escape_htmltag around in file '.$file['fullname'].' ('.$val[1].'$_SERVER[\'PHP_SELF\']). Bad.');
+            */
         }
 
         return;