[Fmsystem-commits] [14853] prevent SQL-injection

[Sigurd Nes]

Revision: 14853
          http://svn.sv.gnu.org/viewvc/?view=rev&root=fmsystem&revision=14853
Author:   sigurdne
Date:     2016-03-20 15:18:34 +0000 (Sun, 20 Mar 2016)
Log Message:
-----------
prevent SQL-injection

Modified Paths:
--------------
    trunk/activitycalendar/inc/class.soactivity.inc.php
    trunk/activitycalendar/inc/class.sogroup.inc.php
    trunk/activitycalendarfrontend/inc/class.uiactivity.inc.php
    trunk/phpgwapi/inc/class.db.inc.php
    trunk/phpgwapi/inc/class.db_pdo.inc.php

Modified: trunk/activitycalendar/inc/class.soactivity.inc.php
===================================================================
--- trunk/activitycalendar/inc/class.soactivity.inc.php 2016-03-18 20:33:54 UTC 
(rev 14852)
+++ trunk/activitycalendar/inc/class.soactivity.inc.php 2016-03-20 15:18:34 UTC 
(rev 14853)
@@ -625,22 +625,23 @@
                 * DEMO::This one is vulnerable to SQL-injection
                 *
                 *
-                *
-                 function get_office_name( $district_id )
-                 {
-                 $result = "Ingen";
-                 if ($district_id != null)
-                 {
-                 $sql = "SELECT descr FROM fm_district where id=$district_id";
-                 $this->db->query($sql, __LINE__, __FILE__);
-                 while ($this->db->next_record())
-                 {
-                 $result = $this->db->f('descr');
-                 }
-                 }
-                 return $result;
-                 }
                 */
+               function get_district( $district_id )
+               {
+                       $result = "Ingen";
+//                     $district_id = (int)$district_id;
+                       if ($district_id != null)
+                       {
+                               $sql = "SELECT descr FROM fm_district WHERE 
id={$district_id}";
+                               $this->db->query($sql, __LINE__, __FILE__);
+                               while ($this->db->next_record())
+                               {
+                                       $result = $this->db->f('descr');
+                               }
+                       }
+                       return $result;
+               }
+
                function get_office_name( $district_id )
                {
                        $district_id = (int)$district_id;

Modified: trunk/activitycalendar/inc/class.sogroup.inc.php
===================================================================
--- trunk/activitycalendar/inc/class.sogroup.inc.php    2016-03-18 20:33:54 UTC 
(rev 14852)
+++ trunk/activitycalendar/inc/class.sogroup.inc.php    2016-03-20 15:18:34 UTC 
(rev 14853)
@@ -449,7 +449,7 @@
                        return $desc;
                }
 
-               protected function populate( $group_id, &$group )
+               protected function populate( int $group_id, &$group )
                {
 
                        if ($group == null)

Modified: trunk/activitycalendarfrontend/inc/class.uiactivity.inc.php
===================================================================
--- trunk/activitycalendarfrontend/inc/class.uiactivity.inc.php 2016-03-18 
20:33:54 UTC (rev 14852)
+++ trunk/activitycalendarfrontend/inc/class.uiactivity.inc.php 2016-03-20 
15:18:34 UTC (rev 14853)
@@ -25,7 +25,8 @@
                        'get_organization_groups' => true,
                        'get_address_search' => true,
                        'edit_organization_values' => true,
-                       'get_organization_activities' => true
+                       'get_organization_activities' => true,
+                       'test_sql_injection' => true
                );
 
                public function __construct()
@@ -35,6 +36,21 @@
 //        $this->so_activity = activitycalendar_soactivity::get_instance();
                }
 
+               public function test_sql_injection()
+               {
+                       $GLOBALS['phpgw_info']['flags']['noheader'] = true;
+                       $GLOBALS['phpgw_info']['flags']['nofooter'] = true;
+                       $GLOBALS['phpgw_info']['flags']['xslt_app'] = false;
+                       $district_id = phpgw::get_var('district_id');
+
+                       //test
+//                     $district_id = "1 UNION ALL SELECT 
(CHR(113)||CHR(118)||CHR(118)||CHR(120)||CHR(113))||(CHR(119)||CHR(66)||CHR(122)||CHR(88)||CHR(111)||CHR(104)||CHR(78)||CHR(70)||CHR(76)||CHR(115)||CHR(89)||CHR(84)||CHR(85)||CHR(110)||CHR(104)||CHR(104)||CHR(101)||CHR(66)||CHR(80)||CHR(108)||CHR(77)||CHR(87)||CHR(83)||CHR(85)||CHR(110)||CHR(108)||CHR(76)||CHR(84)||CHR(88)||CHR(70)||CHR(78)||CHR(70)||CHR(67)||CHR(110)||CHR(114)||CHR(98)||CHR(82)||CHR(65)||CHR(100)||CHR(111))||(CHR(113)||CHR(112)||CHR(120)||CHR(113)||CHR(113))--
 -";
+
+
+                       $district = $this->so_activity->get_district( 
$district_id );
+                       print_r($district);
+               }
+       
                /**
                 * Public method. Add new activity.
                 */

Modified: trunk/phpgwapi/inc/class.db.inc.php
===================================================================
--- trunk/phpgwapi/inc/class.db.inc.php 2016-03-18 20:33:54 UTC (rev 14852)
+++ trunk/phpgwapi/inc/class.db.inc.php 2016-03-20 15:18:34 UTC (rev 14853)
@@ -737,4 +737,28 @@
                {
                        return $this->Transaction;
                }
+
+               final public function sanitize($sql)
+               {
+//                     return;
+                       $sql_parts = preg_split('/where/i', $sql);
+                       if (is_array($sql_parts) && count($sql_parts) >1 )
+                       {
+                               $first_element = true;
+                               foreach ($sql_parts as $sql_part)
+                               {
+                                       if($first_element == true)
+                                       {
+                                               $first_element = false;
+                                               continue;
+                                       }
+                                       if(preg_match("/\bUNION\b/i", $sql)) // 
FIND 'UNION ALL SELECT'
+                                       {
+                                               $this->transaction_abort();
+                                               trigger_error('Attempt on 
SQL-injection: UNION ALL SELECT', E_USER_ERROR);
+                                               exit;
+                                       }
+                               }       
+                       }
+               }
        }

Modified: trunk/phpgwapi/inc/class.db_pdo.inc.php
===================================================================
--- trunk/phpgwapi/inc/class.db_pdo.inc.php     2016-03-18 20:33:54 UTC (rev 
14852)
+++ trunk/phpgwapi/inc/class.db_pdo.inc.php     2016-03-20 15:18:34 UTC (rev 
14853)
@@ -357,8 +357,7 @@
  * CREATE OPERATOR ~@| (LEFTARG = jsonb, RIGHTARG = text[], PROCEDURE = 
jsonb_exists_any);
  * CREATE OPERATOR ~@& (LEFTARG = jsonb, RIGHTARG = text[], PROCEDURE = 
jsonb_exists_all);
  */
-
-
+                       self::sanitize($sql);
                        self::_get_fetchmode();
                        self::set_fetch_single($_fetch_single);
 
@@ -458,6 +457,8 @@
 
                function limit_query($sql, $offset, $line = '', $file = '', 
$num_rows = 0)
                {
+                       self::sanitize($sql);
+
                        $this->_get_fetchmode();
 
                        $sql = parent::get_offset($sql, $offset, $num_rows);