Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix SQLi in location api #654
* Removes Location_MOdel::get_locations since it better to use ORM
* Filter locations by location_visible
* Sanitize location id (automatic thanks to query builder/ORM)
* Slight change to JSON response, but should be backwards compatible
  * Now returns lat/long/id as numbers not strings
  * Now return name and location_name
  * Response now includes location_date
  • Loading branch information
rjmackay committed Jul 2, 2012
1 parent 00eae4f commit a11d43c
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 57 deletions.
15 changes: 11 additions & 4 deletions application/libraries/api/MY_Locations_Api_Object.php
Expand Up @@ -47,9 +47,7 @@ public function perform_task()
}
else
{
$this->response_data = $this->_get_locations(array(
'id = '.$this->request['id']
));
$this->response_data = $this->_get_locations(array('id' => $this->request['id']));
}
break;

Expand Down Expand Up @@ -83,7 +81,12 @@ public function perform_task()
private function _get_locations($where = array())
{
// Fetch the location items
$items = Location_Model::get_locations($where, $this->list_limit);
$items = ORM::factory('Location')
->select('location_name AS name', 'location.*') // Add extra name field for backwards compat
->where($where)
->where('location_visible', 1)
->limit($this->list_limit)
->find_all();

//No record found.
if ($items->count() == 0)
Expand All @@ -99,6 +102,10 @@ private function _get_locations($where = array())

foreach ($items as $item)
{
$item = $item->as_array();
// Hide variables we don't want publicly exposed
unset($item['location_visible']);

// Needs different treatment depending on the output
if ($this->response_type == 'json')
{
Expand Down
53 changes: 0 additions & 53 deletions application/models/location.php
Expand Up @@ -33,59 +33,6 @@ class Location_Model extends ORM
* @var string
*/
protected $table_name = 'location';

/**
* Gets the list of all locations
*
* @param array $where Key value array with extra predicates for the query
* @param int $limit Number of records to fetch
* @return Result
*/
public static function get_locations($where = array(), $limit = 0)
{
// Database table prefix
$table_prefix = Kohana::config('database.default.table_prefix');

// SQL query
$sql = 'SELECT id, location_name AS name, country_id, latitude, longitude '
. 'FROM '.$table_prefix.'location '
. 'WHERE location_visible = 1 ';

// Check for parameters
if ( ! empty($where) AND count($where) > 0)
{
foreach ($where as $column => $value)
{
if ($predicate_items = explode("=", $value))
{
if (count($predicate_items) == 2)
{
$column = $predicate_items[0];
$value = $predicate_items[1];
}
else
{
// Exception handling
throw new Kohana_Exception('Invalid value in "where" parameter');
}
}

$sql .= 'AND '.$column.' = '.$value.' ';
}
}

// Order the records by database ID
$sql .= 'ORDER BY id DESC ';

// Check if the record limit has been specified
if ((int)$limit > 0)
{
$sql .= 'LIMIT 0, '.$limit;
}

$db = new Database();
return $db->query($sql);
}

/**
* Checks if a location id exists in the database
Expand Down

0 comments on commit a11d43c

Please sign in to comment.