Why is my numbering suddenly skipping in my php, mysql application

[JUSTMELAT]

We have a php application that is just a form, that takes in data to a mysql database and then displays that data in queue(php web page) for the client. It has been working for the past 5  or more years. Six weeks ago, the client noticed that the numbering started skipping a number each time a request was entered.  It then seemed to stop, now it has started again.  See attached image.

 

Some detail here:  after user has completed web form and hits submit, three fields are populated in the mysql "REQUEST" table, then the actually data from the submitted web form is sent to the ANSWER table.  The REQUEST table is in sync and is always populated, and numbering is in sequence but upon submit, the actual data from the request does NOT always go to the ANSWER table hence the "skipping of numbers."

     

What do I even begin looking at/for to determine what happened?  Reminder: this is a 5+ year old app that worked fine until maybe 6 weeks ago.  No changes have been made to the database or the code base since 2008.  The code that adds the data to the ANSWER table [the one that appears to skip] is below.  The applicate is in php 4.3.1, mysql 5, linux server.

 

[attachment deleted by admin]

[BlueSkyIS]

the actual data from the request does NOT always go to the ANSWER table hence the "skipping of numbers."

 

based on the code snippet provided, it appears there is no PHP validation of the data: simply submit blank values, and blank values will be inserted into the database.

[Psycho]

Here is the code you attached:

//Add the answers to the Answer table
while (list($key,$val) = each($_REQUEST))
{
    if (preg_match("/^Q/",$key))
    {
        if (is_array($_REQUEST[$key]))
        {
            for ($i=0;$i<count($_REQUEST[$key]);$i++)
            {
                if (!empty($_REQUEST[$key][$i]))
                {
                    $sql = "INSERT INTO ANSWER (Q_ID,TEXT,R_NUMBER) VALUES";
                    $sql .= "('" . $key . "[]','" . addslashes($_REQUEST[$key][$i]) . "','" . $requestNumber . "')";
                    mysql_query($sql,$db) or die("ERROR: " . mysql_error());
                }
            }
        }
        else
        {
            if (!empty($_REQUEST[$key]))
            {
                $sql = "INSERT INTO ANSWER (Q_ID,TEXT,R_NUMBER) VALUES";
                $sql .= "('" . $key . "','" . addslashes($_REQUEST[$key]) . "','" . $requestNumber . "')";
                mysql_query($sql,$db) or die("ERROR: " . mysql_error());
            }
        }
    }

 

Looking at that there are quite a few condition statements that must pass in order for one of the two INSERT queries to be run. Assuming that code is always called after a record is inserted into the REQUEST table I would assume that one fo the conditions is failing. The possible causes I see are as follows:

 

1. The WHILE loop is not running because $_REQUEST has no values.

2. The preg_match("/^Q/",$key) condition test fails

3. When $_REQUEST[$key] is an array the !empty($_REQUEST[$key][$i]) condition is failing

3. When $_REQUEST[$key] is not an array the !empty($_REQUEST[$key]) condition is failing.

 

I think you should first start looking at #1. Using $_REQUEST is not recommended (google to find out why). That global variable "typically" holds the contents of $_GET, $_POST and $_COOKIE. But, according to the manual

...The presence and order of variables listed in this array is defined according to the PHP variables_order configuration directive

 

So, if that server configuration changed, your code may not functiona correctly. It is really bad practice to use all $_REQUEST values for running db queries. That code will run against all those variables - NOT just the form input. All it would take is to add some JavaScript code to add a cookie for what would seem to be a benign feature and that code would do some very odd things. You should probably change that code to use $_POST or $_GET depending upon the method used by the form. Whichever variable you are using you can test it by printing its contents to the page to validate the values.

 

You could also do the same for the condition statements by echoing to the page whenthe conditions pass/fail to understand what is going on.

 

I have modified your code below to add a simple debugging feature that you can turn on/off to help find the cause. Note: I would typically make this turn on/off based upon adding a parameter to the URL so you can test it in a production environment without affecting the users, but since you are using the $_REQUEST object that is out of the question.

<?php

//Change to false to suppress debug messages
$debug = true;
$debugTxt = ''

//Add the answers to the Answer table
$debugTxt .= "Contents of REQUEST:\n" . print_r($_REQUEST) . "\n\n";
while (list($key,$val) = each($_REQUEST))
{
    $debugTxt .= "<b>Processing key {$key}:</b>\n";
    if (preg_match("/^Q/",$key))
    {
        $debugTxt .= " - preg_match(/^Q/) passed\n";
        if (is_array($_REQUEST[$key]))
        {
            $debugTxt .= " - Is an array\n";
            for ($i=0; $i<count($_REQUEST[$key]); $i++)
            {
                if (!empty($_REQUEST[$key][$i]))
                {
                    $debugTxt .= " - Is NOT empty\n";
                    $sql = "INSERT INTO ANSWER (Q_ID,TEXT,R_NUMBER) VALUES";
                    $sql .= "('" . $key . "[]','" . addslashes($_REQUEST[$key][$i]) . "','" . $requestNumber . "')";
                    mysql_query($sql,$db) or die("ERROR: " . mysql_error());
                    $debugTxt .= " - Query: {$sql}\n";
                }
                else
                {
                    $debugTxt .= " - Is empty\n"
                }
            }
        }
        else
        {
            $debugTxt .= " - Is NOT an array\n";
            if (!empty($_REQUEST[$key]))
            {
                $debugTxt .= " - Is NOT empty\n";
                $sql = "INSERT INTO ANSWER (Q_ID,TEXT,R_NUMBER) VALUES";
                $sql .= "('" . $key . "','" . addslashes($_REQUEST[$key]) . "','" . $requestNumber . "')";
                $debugTxt .= " - Query: {$sql}\n";
                mysql_query($sql,$db) or die("ERROR: " . mysql_error());
            }
            else
            {
                $debugTxt .= " - Is empty\n"
            }
        }
    }
    else
    {
        $debugTxt .= "preg_match(/^Q/) failed\n";
    }
}

echo "<pre>$debugTxt</pre>";
?>

 

Also, I was going to make a change for the branch of code for processing arrays. You should not be running queries in loops. Instead, get allt he records to add and run a single query. But, I dodn't want to change the core logic of your code until you have a change to find the cause.

[JUSTMELAT]

Hi mjdamato

 

Thanks so much for your suggestion.  I had no idea where to start particular since this application has been running for so long without an issue.  I will spend tomorrow run through your suggestions.  Thanks again.  I will let you know what I find.

[xylex]

You also have that sql injection vulnerability in there you should probably take care of. 

 

while (list($key,$val) = each($_REQUEST))
{
    if (preg_match("/^Q/",$key))
    {
                    $sql = "INSERT INTO ANSWER (Q_ID,TEXT,R_NUMBER) VALUES";
                    $sql .= "('" . $key . "[]','" . addslashes($_REQUEST[$key][$i]) . "','" . $requestNumber . "')";
    }
}

 

Need to do more validation than making sure $key starts with a Q. 

[Psycho]

You also have that sql injection vulnerability in there you should probably take care of. 

 

You are correct, but addslashes() is not what you should be using. Instead use mysql_real_escape_string(). Per the manual for addslashes() it states:

It's highly recommended to use DBMS specific escape function (e.g. mysqli_real_escape_string() for MySQL or pg_escape_string() for PostgreSQL),

[BlueSkyIS]

... not to mention that magic_quotes may have already added slashes, which would leave slashes in the data. I like to check for magic_quotes and use stripslashes() if necessary before using mysql_real_escape_string

 

$some_field = $_POST['some_field'];

if (get_magic_quotes_gpc())  {
$some_field = stripslashes($some_field);
}

$some_field = mysql_real_escape_string($some_field);

[xylex]

I was just quoting the snippet of the code that has the injection vulnerability, I didn't change anything other than taking out the code not relevant to what I was talking about. $key is the $_POST key from the client, and the code only checks that it starts with 'Q' before using it totally unescaped in the database.