Author: JS

It wasn’t obvious to me that you need to have a different bindParameter statement each time you use a variable in the query. This won’t work.

// query for title information
$qry  = "SELECT * ";
$qry .= "FROM `website_database`.`product`, `website_database`.`product_instance` ";
$qry .= "WHERE product.id = :productID ";
$qry .= "AND product_instance.product_id = :productID";

$stmt = $dbWG->prepare($qry);
$stmt->bindParam(':productID', $productID);
$stmt->execute();

MySql doesn’t look for every instance of :productID in the query and substitute $productID. This is how you do it:

// query for title information
$qry  = "SELECT * ";
$qry .= "FROM `website_database`.`product`, `website_database`.`product_instance` ";
$qry .= "WHERE product.id = :productID1 ";
$qry .= "AND product_instance.product_id = :productID2";

$stmt = $dbWG->prepare($qry);
$stmt->bindParam(':productID1', $productID);
$stmt->bindParam(':productID2', $productID);
$stmt->execute();

As part of my site rewrite and migration to MySQL PDO I am making sure that all of the input is sanitized before using—which has the side effect of stopping injection attempts, as discussed in the previous post—and either using prepared statements or whitelisted inputs.

Here’s the sanitizing portion of a crossword solver page. The input is the number of letters in the word and up to 14 letters. There should only be one letter in each space, the space can be empty, and the numbers can be from 1 to 14. I haven’t had any attacks on my forms yet, so I’ll assume any invalid input is due to fat fingers and make reasonable changes.

<?php
$MAX_LETTERS = 14;
$letters = array();

// Read in the letters and number of letters first so you can repopulate the fields.
// If it’s not a single letter in the letter field, or a valid number in the number field,
// don’t let it into the query.
// Log it in case we get lots of injection attempts.
if(isset($_POST)) {
    $submitType = $_POST['submitType'];

    if ($submitType != 'Clear') {
        // Get and validate letters
        for($i = 1; $i <= $MAX_LETTERS; $i++) {
            $letters[$i] = $_POST['letter' . $i];
            $letters[$i] = str_replace(" ","",$letters[$i]);

            if (!preg_match("/^[a-zA-Z]$/",$letters[$i]) && ($letters[$i] <> '') ) {
                if ($showError) error_log("Not a letter {$letters[$i]} in $calledFileName");
                $letters[$i] = "";
            }
        }
        // Get and validate the number of letters
        $num_letters = (integer)$_POST['num_letters'];
        if (!is_integer($num_letters) ) {
            if ($showError) error_log("Not a number in $calledFileName");
        }
        $num_letters = (integer)$num_letters;
        if ( $num_letters > $MAX_LETTERS ) {
            $num_letters = $MAX_LETTERS;
            if ($showError) error_log("Too many numbers in $calledFileName");
        } else if ($num_letters < 0 ) {
            $num_letters = $num_letters * -1;
            if ($showError) error_log("Negative number in $calledFileName");
        }
    }
}

Note that I don’t use htmlspecialchars or mysql_real_escape_string when getting input because I explicitly allow only letters or numbers when validating the output. I don’t think that they would hurt anything, but they aren’t necessary.

$letters[$i] = htmlspecialchars($_POST['letter' . $i]);
$letters[$i] = mysql_real_escape_string($_POST['letter' . $i]);

Some of my pages allow more than one letter in each input space. I just add .* to the pre_match to allow more than one letter. I also allow a wildcard, *, in the web page so I need to escape it in the pre_match.

if (!preg_match("/[a-zA-Z\*].*/",$letters[$i]) && ($letters[$i] <> '') ) {

The safest way to sanitize input is to whitelist the query. There are lots of ways to do this. One way is to construct the query based on the input.

switch ($loc) {
    case "I":
    case "i":
        $location = "Initial";
        $searchString = "^{$letters}[A-Z ]*";
        break;
    case "M":
    case "m":
        $location = "Medial";
        $searchString = "[A-Z ]+{$letters}[A-Z ]+";
        break;
    case "F":
    case "f":
        $location = "Final";
        $searchString = "[A-Z ]*{$letters}\$";
        break;
.....

Another example of whitelisting is to only allow certain values. It works if the list is small and not changed often.

if ($input == 'first value' || $input == 'second value' || $input == 'third value') {
    -- do stuff with the database
} else {
   include_once('dieInAFire');
}

You can do something similar by checking whether the input is part of a hard-coded array. The problem with the last two approaches is that they only work well if the list of acceptable values doesn’t change often. They can work to sanitize user input for things like states and provinces, occupation, and taxable status.

As I discussed in an earlier post, my sites are frequently attacked by malicious actors trying to get into my database by using SQL injection techniques. I stopped most of them by doing input validation and if the input fails validation, returning a 404 not found. I didn’t do this on all of my pages so from time to time they find a page that does”t fail with bad input and send thousands of page requests to it.

I recently updated my sites and took the opportunity to familiarize myself with the latest best practices for mitigating attacks. I added a line to my database initialization script, sanitize all inputs, and use either prepared statements or queries that use whitelisted terms. Here’s my current initialization script.

<?php

$user = 'mysql_user';
$pass = 'supersecurepasswors';
$host = 'localhost';
$db_name = 'website_database';

try {
    $dbWG = new PDO("mysql:host=$host;db_name=$dbname", $user, $pass, array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8") );
    $dbWG->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
    $dbWG->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
    $dbWG->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
}
catch(PDOException $e) {
    $e->getMessage();

The only way I found to get attacks to stop is to return a 404 not found message to the bot. After a few tries they usually give up. Otherwise they will try thousands of times.

So first I sanitize the input. Here’s a simple one where the input has to be a digit:

$number_of_letters = (isset($_GET['n']) ? $_GET['n'] : 1);
if ( !is_numeric($number_of_letters) ) {
    include_once('dieInAFire.inc');
}

I have a $showError variable in my header that lets me turn off and on information on attacks. I usually leave it on and peruse the error log to make sure legitimate requests aren’t accidentally blocked. I wrote this include file to use after sanitizing inputs fails.

<?php
$url = isset($_SERVER['REQUEST_URI'])  ? $_SERVER['REQUEST_URI']  : '';
$ref = isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : '';
$ip  = isset($_SERVER['REMOTE_ADDR'])  ? $_SERVER['REMOTE_ADDR']  : '';

if ($showError == TRUE) {
    error_log("Long string in $calledFileName: URL is $url and IP is $ip & ref is $ref");
}
header("HTTP/1.0 404 Not Found");
die();
?>

Note that this has to be called before the statement.

If it is a legitimate request, I make then use a prepared statement to execute it.

$qry.=  "HAVING COUNT(sorted_letters) > :number_of_letters ";

$stmt = $dbWG->prepare($qry);
$stmt->bindParam(':number_of_letters', $number_of_letters);
$stmt->execute();