SQL injection

[TheTitans]

Let's say I have something like this:

 

if (isset($_GET['id']) && $_GET['id'] != '') { // If '?id=' exists in the URL
$id = validationCheck($_GET['id']);
if ($id) {
	// do stuff here
}
}

function validationCheck($input) {
if (get_magic_quotes_gpc() || get_magic_quotes_runtime()) {
    	$input = stripslashes($input);
}
$input = mysql_real_escape_string($input);
return $input;
}

 

Is that function I have good enough to prevent me from SQL injection?

[PFMaBiSmAd]

 

 

Yes, but only if the datatype is a string, which is why the function is called mysql_real_escape_string. For numeric data, it does not protect against sql injection. Also, get_magic_quotes_runtime() has nothing to do with GET/POST/COOKIE data so it should not be part of that code and since you can turn off magic_quotes_runtime in your script, it is simpler to turn it off than to write code to test it.

 

For your example, using an id, you probably have a query like this -

 

$query = "SELECT something FROM your_table WHERE id = $id";

 

Since the variable $id is not within quotes for a query like that (using best programming practices), it is possible to inject a UNION query to display all the information in your table after that select query by supplying a number to satisfy the id = $id part of the query then inject sql after that.

 

For numeric data, you either need to cast it as a number using (int) or validate that it only contains numeric digits.

[jonsjava]

here's something I threw together for you. It's *NOT* the most secure thing, but it's a good base for you to work off of. I have a better version, but how would that help you learn?

<?php

/**
* This function will take input (be it a variable, or a single-dimensional array), and make it safe for databases. this was written quickly
* so it won't be completely secure, but it's something to start with.
*
* USAGE: $variable = clean_input($input_data, false, true);
* This would clean the input $input_data, and state it does not need to be numeric, and it *is* from a $_GET
* 
* 
* @param unknown_type $data : The data you are wanting to evaluate.
* @param unknown_type $is_numeric : Is this a number? set it to true if it is, or false if not.
* @param unknown_type $is_get : is this from a $_GET? if so, set to true. defaults to false
*/
function clean_input($data, $is_numeric=false, $is_get=false){
if (is_array($data)){
	foreach ($data as $key=>$value){
		$temp_array[$key] = mysql_real_escape_string($value);
	}
	return $temp_array;
}
else{
	if ($is_numeric != false){
		if (!is_numeric($data)){
			return false;
		}
		else{
			return $data;
		}
	}
	elseif ($is_get != false){
		$temp_data = mysql_real_escape_string($data);
		if ((stristr($data, "http")) || (stristr($data, "www."))){
			return false;
		}
		else{
			return $data;
		}
	}
	else{
		$temp_data = mysql_real_escape_string($data);
		return $temp_data;
	}
}
}
?>

[sniperscope]

Use this code. save it and call it anywhere of you application. hope it helps

<?php

// ----------------------------------------------------------------------------
//
// class.floodblocker.php - FloodBlocker class, ver.0.01 (April 15, 2005)
//
// Description:
//   Class allowing to protect the scripts from flooding and to prevent
//   automatic download of the site from single IP.
//
// Author:
//   Vagharshak Tozalakyan <vagh@armdex.com>
//   This module was written by author on its leasure time.
//
// Warning:
//   This class is non commercial, non professional work. It should not have
//   unexpected results. However, if any damage is caused by this class the
//   author can not be responsible. The use of this class is at the risk of
//   the user.
//
// Requirements:
//   PHP >= 4.1.0
//
// ----------------------------------------------------------------------------


// Errors and warnings

define ( 'E_TMP_DIR',    'Incorrect temprorary directory specified.' );
define ( 'E_IP_ADDR',    'Incorrect IP address specified.' );
define ( 'E_LOG_FILE',   'Log file access error! Check permissions to write.' );
define ( 'E_CRON_FNAME', 'The name of cron file must begin with dot.' );
define ( 'E_CRON_FILE',  'Cron file access error! Check permissions to write.' );
define ( 'E_CRON_JOB',   'Unable to perform the cron job.' );


// Class definition

class FloodBlocker
{

  // The directory where log files will be saved. Must have permissions to write.
  var $logs_path;

  // IP address of current connection. REMOTE_ADDR will be used by default.
  var $ip_addr;

  // An associative array of [$interval=>$limit] format, where $limit is the
  // number of possible requests during $interval seconds.
  var $rules;

  // The name of the cron file. Must begin with dot. Default filename is '.time'.
  var $cron_file;

  // Cron execution interval in seconds. 1800 secs (30 mins) by default.
  var $cron_interval;

  // After how many of seconds to consider a file as old? By default the files
  // will consider as old after 7200 secs (2 hours).
  var $logs_timeout;


  /*
    Description:
      Class constructor.
    Prototype:
      void FloodBlocker ( string logs_path, string ip = '' )
    Parameters:
      logs_path - the directory where log files will be saved
      ip - the ip address of the current connection,
           $_SERVER['REMOTE_ADDR'] will be used if ip=''
  */
  function FloodBlocker ( $logs_path, $ip = '' )
  {

    if ( ! is_dir ( $logs_path ) )
      trigger_error ( E_TMP_DIR, E_USER_ERROR );

    $logs_path = str_replace ( '\\', '/', $logs_path );
    if ( substr ( $logs_path, -1 ) != '/' )
      $logs_path .= '/';

    $this->logs_path = $logs_path;

    if ( empty ( $ip ) )
      $ip = $_SERVER['REMOTE_ADDR'];

    $ip = ip2long ( $ip );
    if ( $ip == -1 || $ip === FALSE )
      trigger_error ( E_IP_ADDR, E_USER_ERROR );

    $this->ip_addr = $ip;

    $this->rules = array ( );
    $this->cron_file = '.time';
    $this->cron_interval = 1800;  // 30 minutes
    $this->logs_timeout = 7200;  // 2 hours

  }


  /*
    Description:
      Used to check flooding. Generally this function acts as private method
      and will be called internally by public methods. However, it can be called
      directly when storing logs in db.
    Prototype:
      bool RawCheck ( array &info )
    Parameters:
      info - $interval=>$time, $interval=>$count array
    Return:
      FALSE if flood detected, otherwise - TRUE.
  */
  function RawCheck ( &$info )
  {

    $no_flood = TRUE;

    foreach ( $this->rules as $interval=>$limit )
    {
      if ( ! isset ( $info[$interval] ) )
      {
        $info[$interval]['time'] = time ( );
        $info[$interval]['count'] = 0;
      }

      $info[$interval]['count'] += 1;

      if ( time ( ) - $info[$interval]['time'] > $interval )
      {
        $info[$interval]['count'] = 1;
        $info[$interval]['time'] = time ( );
      }

      if ( $info[$interval]['count'] > $limit )
      {
        $info[$interval]['time'] = time ( );
        $no_flood = FALSE;
      }

      // The following two lines can be used for debugging
      // echo $info[$interval]['count'].'  ';
      // echo $info[$interval]['time'].'<br>';

    }  // foreach

    return $no_flood;

  }


  /*
    Description:
      Checks flooding. Must be called after setting up all necessary properties.
    Prototype:
      bool CheckFlood ( )
    Return:
      FALSE if flood detected, otherwise - TRUE.
  */
  function CheckFlood ( )
  {

    $this->CheckCron ( );

    $path = $this->logs_path . $this->ip_addr;

    if ( ! ( $f = fopen ( $path, 'a+' ) ) )
      trigger_error ( E_LOG_FILE, E_USER_ERROR);

    flock ( $f, LOCK_EX );

    $info = fread ( $f, filesize ( $path ) + 10 );
    $info = unserialize( $info );

    $result = $this->RawCheck ( $info );

    ftruncate ( $f, 0 );
    fwrite ( $f, serialize( $info ) );
    fflush ( $f );

    flock($f, LOCK_UN);

    fclose($f);

    return $result;

  }


  /*
    Description:
      Checks the cron file and calls CronJob() to delete old entries from logs
      directory if the time-out is reached.
    Prototype:
      void CheckCron ( )
  */
  function CheckCron ( )
  {

    if ( substr ( $this->cron_file, 0, 1 ) != '.' )
    {
      trigger_error ( E_CRON_FNAME, E_USER_WARNING );
      return;
    }

    $path = $this->logs_path . $this->cron_file;

    if ( ! ( $f = fopen ( $path, 'a+' ) ) )
    {
      trigger_error ( E_CRON_FILE, E_USER_WARNING );
      return;
    }

    flock ( $f, LOCK_EX );

    $last_cron = fread ( $f, filesize ( $path ) + 10 );
    $last_cron = abs ( intval ( $last_cron ) );

    if ( time ( ) - $last_cron > $this->cron_interval )
    {
      $this->CronJob ( );
      $last_cron = time ( );
    }

    ftruncate ( $f, 0 );
    fwrite ( $f, $last_cron );
    fflush ( $f );

    flock ( $f, LOCK_UN );

    fclose ( $f );

  }


  /*
    Description:
      Deletes all old files from logs directory, except the files starting
      with dot.
    Prototype:
      void CronJob ( )
  */
  function CronJob ( )
  {

    $path = $this->logs_path;

    if ( ! ( $dir_hndl = opendir ( $this->logs_path ) ) )
    {
      trigger_error ( E_CRON_JOB, E_USER_WARNING);
      return;
    }

    while ( $fname = readdir ( $dir_hndl ) )
    {
      if ( substr( $fname, 0, 1 ) == '.' )
        continue;
      clearstatcache ( );
      $ftm = filemtime ( $path . $fname );
      if ( time ( ) - $ftm > $this->logs_timeout )
        @unlink ( $path . $fname );
    }

    closedir ( $dir_hndl );

  }

}  // end of class definition



/*
  $flb = new FloodBlocker ( 'example/tmp-ips/' );
  $flb->rules = array ( 10=>5 );
  $res = $flb->CheckFlood ( );
  if ( $res )
    echo 'Succeed!';
  else
    die ( 'Too many requests! Please try later.' );
*/

?>

[PFMaBiSmAd]

 

 

Too bad there is nothing in that code that has anything to do with protecting data in a query from sql injection.

[sniperscope]

 

 

Too bad there is nothing in that code that has anything to do with protecting data in a query from sql injection.

I was just trying to help 

[premiso]

 

 

Too bad there is nothing in that code that has anything to do with protecting data in a query from sql injection.

I was just trying to help 

 

It's ok to try to help as long as it is related to the topic. That code is not really related to the topic, it has nothing to do with MySQL and protecting the site from being SQL Injected.

 

Sorry to be rude, but yea relevance to a topic is always a good rule to follow.