HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

SQL-injection mitigation script

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sqlmitigationscriptinjection

Problem

I'll gladly appreciate it if you could review my code below and let me know if they are sufficiently secure.

My main website and these scripts will use same database, so I need to make sure they are sufficiently secure before I put them live.

Since I cannot post more than two links, I'll post my Anti-SQL Injection script, and below are the actual two scripts.

 0) {                      
    die("SQL Эnjection Denemesi Yaptэnэz... ama Baюarэsэz Oldunuz ;)");  
    }
  }
 }
}
sql_inject_chec();
?>


Ingame registration - SCRIPT

Score display - SCRIPT

Solution

This is not a constructive way to protect your scripts from SQL injection — the objective of SQL-injection prevention should not be to disallow characters or words, but to escape characters. What you have now, prevents your users from submitting perfectly valid English words such as select, update, etc. and/or perfectly valid characters such as --, " and '.

Consider this fictional comment:


When I select "where" from the menu, it updates the procedure -- strange.

This would be rejected by your code.

Now, I'm sure, for your application, this might (currently) be irrelevant, but your code is not very flexible or portable. What happens if your current application, or another future application needs to accept these words? You'd have to rewrite your code, and for a possible new application you wouldn't be able to copy existing code.

More importantly though, your current code is probably not elaborate enough to mitigate all possible attack vectors. You'd really be wise to implement prepared, parameterized statements by using PDO, instead of trying to prevent your users from entering possible SQL keywords and characters. With prepared, parameterized statements you don't have to worry about escaping the correct characters anymore — PDO (or, if the driver supports it natively, the database driver) will escape the parameter values you supply for you.

Let me give you a short introductory example of how you could implement SQL-injection prevention with PDO:

// create a database connection handle
$driver   = 'odbc';
$database = 'kn_online';
$username = '*redacted*';
$password = '*redacted*';
$dbh = new PDO( "$driver:$database", $username, $password );

// create a parameterized statement to select a record from CURRENTUSER
// uses a named parameter (denoted by :), called accountId
// that will be properly escaped by PDO
$sql = 'SELECT * FROM CURRENTUSER WHERE StrAccountID = :accountId';

// prepare the statement; returns a PDOStatement object
$stmt = $dbh->prepare( $sql );

// execute the statement by passing the parameter values in an associative array
$executionResult = $stmt->execute( array(
    ':accountId' => $acc
) );

// if the execution succeeded
if( $executionResult )
{
    // fetch the record as an associative array
    $record = $stmt->fetch( PDO::FETCH_ASSOC );
}


The beauty of prepared statements is that you can also easily execute them repeatedly, with new parameter values. Consider this example (connection handle creation omitted for brevity):

// insert into some table
$sql = 'INSERT INTO SOMETABLE VALUES( :id, :name )';

$stmt = $dbh->prepare( $sql );

// some fictional data
// parameter value keys don't need : prepended
$data = array(
    array( 'id' => 1, 'name' => 'Name One' ),
    array( 'id' => 2, 'name' => 'Name Two' ),
    array( 'id' => 3, 'name' => 'Name Three' )
);

foreach( $data as $datum )
{
    // using the same prepared statement
    // to execute for each datum
    $stmt->execute( $datum );
}

Code Snippets

// create a database connection handle
$driver   = 'odbc';
$database = 'kn_online';
$username = '*redacted*';
$password = '*redacted*';
$dbh = new PDO( "$driver:$database", $username, $password );

// create a parameterized statement to select a record from CURRENTUSER
// uses a named parameter (denoted by :), called accountId
// that will be properly escaped by PDO
$sql = 'SELECT * FROM CURRENTUSER WHERE StrAccountID = :accountId';

// prepare the statement; returns a PDOStatement object
$stmt = $dbh->prepare( $sql );

// execute the statement by passing the parameter values in an associative array
$executionResult = $stmt->execute( array(
    ':accountId' => $acc
) );

// if the execution succeeded
if( $executionResult )
{
    // fetch the record as an associative array
    $record = $stmt->fetch( PDO::FETCH_ASSOC );
}
// insert into some table
$sql = 'INSERT INTO SOMETABLE VALUES( :id, :name )';

$stmt = $dbh->prepare( $sql );

// some fictional data
// parameter value keys don't need : prepended
$data = array(
    array( 'id' => 1, 'name' => 'Name One' ),
    array( 'id' => 2, 'name' => 'Name Two' ),
    array( 'id' => 3, 'name' => 'Name Three' )
);

foreach( $data as $datum )
{
    // using the same prepared statement
    // to execute for each datum
    $stmt->execute( $datum );
}

Context

StackExchange Code Review Q#33526, answer score: 9

Revisions (0)

No revisions yet.