Vulnerability: Multiple SQL injection vulnerabilities in application/libraries/api/MY_Countries_Api_Object.php in the Ushahidi Platform before 2.5 allow remote attackers to execute arbitrary SQL commands via vectors related to _get_countries functions.
Vulnerability ID: CVE-2012-3470
CWE: CWE-89
Severity score: 6.4
Project: https://github.com/ushahidi/Ushahidi_Web
Published: 2012-08-12
File: application/libraries/api/MY_Countries_Api_Object.php
Commit message: Fix SQLi in MY_Countries_Api_Object.php #645

Converted _get_countries function to use the ORM instead so queries
are sanitized automatically. This adds some extra fields to the response
but existing fields are unchanged.
Commit author: Robbie Mackay

Vulnerable version (before fix):
 public function perform_task()     /**      * Fetch all countries      *      * @param string where - the where clause for sql      * @param string limit - the limit number      * @param string response_type - XML or JSON      *      * @return string       */     private function _get_countries($where = '', $limit = '')     {          // Fetch countries         $this->query = "SELECT id, iso, country as `name`, capital             FROM `".$this->table_prefix."country` $where $limit";          $items = $this->db->query($this->query);                  // Set the record count         $this->record_count = $items->count();                  $i = 0;          $json_countries = array();         $ret_json_or_xml = '';                  //No record found.         if ($items->count() == 0)         {             return $this->response(4);         } private function _get_countries($where = '', $limit = '')             // Needs different treatment depending on the output             if ($this->response_type == 'json')             {                 $json_countries[] = array("country" => $item);             }              else              {                 $json_countries['country'.$i] = array(                         "country" => $item);                  $this->replar[] = 'country'.$i;             } private function _get_countries($where = '', $limit = '')      */     private function _get_countries_by_all()     {         $where = "ORDER by id DESC ";          return $this->_get_countries($where);     }      /** private function _get_countries_by_all()      */     private function _get_country_by_name($name)     {         $where = "\n WHERE country = '$name' ";         $where .= "ORDER by id DESC";         $limit = "\nLIMIT 0, $this->list_limit";                  return $this->_get_countries($where, $limit);     }      /** private function _get_country_by_name($name)      */     private function _get_country_by_id($id)     {         $where = "\n WHERE id=$id ";         $where .= "ORDER by id DESC";         $limit = "\nLIMIT 0, $this->list_limit";                  return $this->_get_countries($where, $limit);     }      /** private function _get_country_by_id($id)      */     private function _get_country_by_iso($iso)     {         $where = "\n WHERE iso='$iso' ";         $where .= "ORDER by id DESC";         $limit = "\nLIMIT 0, $this->list_limit";         return $this->_get_countries($where, $limit);     } }  ?>

Fixed version (after fix):
 public function perform_task()     /**      * Fetch all countries      *      * @param array where - array to pass to query builder      * @param integer limit - number of results to return      *      * @return string       */     private function _get_countries($where = array(), $limit = FALSE)     {          // Fetch countries 				$items = ORM::factory('Country') 						->select('country as name', 'country.*') 						->where($where) 						->orderby('id','DESC'); 				 				if ($limit) 					$items->limit($limit); 				 				$items = $items->find_all(); 				         // Set the record count         $this->record_count = count($items);                  $i = 0;          $json_countries = array();         $ret_json_or_xml = '';          				//No record found.         if ($this->record_count == 0)         {             return $this->response(4);         } private function _get_countries($where = '', $limit = '')             // Needs different treatment depending on the output             if ($this->response_type == 'json')             {                 $json_countries[] = array("country" => $item->as_array());             }              else              {                 $json_countries['country'.$i] = array(                         "country" => $item->as_array());                  $this->replar[] = 'country'.$i;             } private function _get_countries($where = '', $limit = '')      */     private function _get_countries_by_all()     {         return $this->_get_countries();     }      /** private function _get_countries_by_all()      */     private function _get_country_by_name($name)     { 				$where = array('country' => $name);                  return $this->_get_countries($where, $this->list_limit);     }      /** private function _get_country_by_name($name)      */     private function _get_country_by_id($id)     {         $where = array('id' => $id);                  return $this->_get_countries($where, $this->list_limit);     }      /** private function _get_country_by_id($id)      */     private function _get_country_by_iso($iso)     {         $where = array('iso' => $iso);         return $this->_get_countries($where, $this->list_limit);     } } 

Patch:
patch: @@ -90,31 +90,35 @@ public function perform_task()
     /**
      * Fetch all countries
      *
-     * @param string where - the where clause for sql
-     * @param string limit - the limit number
-     * @param string response_type - XML or JSON
+     * @param array where - array to pass to query builder
+     * @param integer limit - number of results to return
      *
      * @return string 
      */
-    private function _get_countries($where = '', $limit = '')
+    private function _get_countries($where = array(), $limit = FALSE)
     {
 
         // Fetch countries
-        $this->query = "SELECT id, iso, country as `name`, capital
-            FROM `".$this->table_prefix."country` $where $limit";
-
-        $items = $this->db->query($this->query);
-        
+				$items = ORM::factory('Country')
+						->select('country as name', 'country.*')
+						->where($where)
+						->orderby('id','DESC');
+				
+				if ($limit)
+					$items->limit($limit);
+				
+				$items = $items->find_all();
+				
         // Set the record count
-        $this->record_count = $items->count();
+        $this->record_count = count($items);
         
         $i = 0;
 
         $json_countries = array();
         $ret_json_or_xml = '';
         
-        //No record found.
-        if ($items->count() == 0)
+				//No record found.
+        if ($this->record_count == 0)
         {
             return $this->response(4);
         }
@@ -125,12 +129,12 @@ private function _get_countries($where = '', $limit = '')
             // Needs different treatment depending on the output
             if ($this->response_type == 'json')
             {
-                $json_countries[] = array("country" => $item);
+                $json_countries[] = array("country" => $item->as_array());
             } 
             else 
             {
                 $json_countries['country'.$i] = array(
-                        "country" => $item);
+                        "country" => $item->as_array());
 
                 $this->replar[] = 'country'.$i;
             }
@@ -168,8 +172,7 @@ private function _get_countries($where = '', $limit = '')
      */
     private function _get_countries_by_all()
     {
-        $where = "ORDER by id DESC "; 
-        return $this->_get_countries($where);
+        return $this->_get_countries();
     }
 
     /**
@@ -180,11 +183,9 @@ private function _get_countries_by_all()
      */
     private function _get_country_by_name($name)
     {
-        $where = "\n WHERE country = '$name' ";
-        $where .= "ORDER by id DESC";
-        $limit = "\nLIMIT 0, $this->list_limit";
+				$where = array('country' => $name);
         
-        return $this->_get_countries($where, $limit);
+        return $this->_get_countries($where, $this->list_limit);
     }
 
     /**
@@ -195,11 +196,9 @@ private function _get_country_by_name($name)
      */
     private function _get_country_by_id($id)
     {
-        $where = "\n WHERE id=$id ";
-        $where .= "ORDER by id DESC";
-        $limit = "\nLIMIT 0, $this->list_limit";
+        $where = array('id' => $id);
         
-        return $this->_get_countries($where, $limit);
+        return $this->_get_countries($where, $this->list_limit);
     }
 
     /**
@@ -209,11 +208,8 @@ private function _get_country_by_id($id)
      */
     private function _get_country_by_iso($iso)
     {
-        $where = "\n WHERE iso='$iso' ";
-        $where .= "ORDER by id DESC";
-        $limit = "\nLIMIT 0, $this->list_limit";
-        return $this->_get_countries($where, $limit);
+        $where = array('iso' => $iso);
+        return $this->_get_countries($where, $this->list_limit);
     }
 }
 
-?>
path: application/libraries/api/MY_Countries_Api_Object.php