[Release] processor.php protection for potential security risk

08/22/2010 14:49 il.mane#1

if you are useing the processor.php script, you need to know that is potentially attackable with code ijections.

Here is a little solution that may help ya to fix SQL code injection, put this code at the beginning of your processor.php

Code:

function sql_quote( $value )
{
    if( get_magic_quotes_gpc() )
    {
          $value = stripslashes( $value );
    }
    //check if this function exists
    if( function_exists( "mysql_real_escape_string" ) )
    {
          $value = mysql_real_escape_string( $value );
    }
    //for PHP version < 4.3.0 use addslashes
    else
    {
          $value = addslashes( $value );
    }
    return $value;
}

08/22/2010 19:55 ProfNerwosol#2

Vulnerable to code injections in what way?

08/22/2010 23:05 abrasive#3

Quote:

Originally Posted by ProfNerwosol

Vulnerable to code injections in what way?

The original script takes whatever the user enters as the "userid" and "pass" and inserts them directly into the queries, no questions asked.

There's actually a large number of problems with the original script, SQL injection being the most severe by far.

Another problem is how the last ID is obtained:

Code:

SELECT Max(UserUID) AS max FROM PS_UserData.dbo.Users_Master

This method leaves it open for a race condition in which two users could get the same max UserUID. This is because the script doesn't know of the existence of any other running instances of itself asking the database for the same thing.

Something like this should be used instead:

Code:

SELECT IDENT_CURRENT('Users_Master')

There is a lot of problems with the database as well: missing primary keys, incorrect identity columns, and missing unique constraints to name a few.

08/23/2010 19:32 jamessimpler#4

Would you good sir make a complete file and make it available for the nub user like myself? Thanks in advance

Quote:
Originally Posted by abrasive
The original script takes whatever the user enters as the "userid" and "pass" and inserts them directly into the queries, no questions asked.

There's actually a large number of problems with the original script, SQL injection being the most severe by far.

Another problem is how the last ID is obtained:
Code:
SELECT Max(UserUID) AS max FROM PS_UserData.dbo.Users_Master
This method leaves it open for a race condition in which two users could get the same max UserUID. This is because the script doesn't know of the existence of any other running instances of itself asking the database for the same thing.

Something like this should be used instead:
Code:
SELECT IDENT_CURRENT('Users_Master')
There is a lot of problems with the database as well: missing primary keys, incorrect identity columns, and missing unique constraints to name a few.

08/24/2010 19:06 abrasive#5

Quote:

Originally Posted by jamessimpler

Would you good sir make a complete file and make it available for the nub user like myself? Thanks in advance

I don't have php set up to connect to mssql to test it. Also after the changes to the database that have been made, whatever I came up with wouldn't work on anyone else's server anyways.

08/24/2010 22:15 ProfNerwosol#6

Quote:
Originally Posted by abrasive
Spoiler
The original script takes whatever the user enters as the "userid" and "pass" and inserts them directly into the queries, no questions asked.

There's actually a large number of problems with the original script, SQL injection being the most severe by far.

Another problem is how the last ID is obtained:
Code:
SELECT Max(UserUID) AS max FROM PS_UserData.dbo.Users_Master
This method leaves it open for a race condition in which two users could get the same max UserUID. This is because the script doesn't know of the existence of any other running instances of itself asking the database for the same thing.

Something like this should be used instead:
Code:
SELECT IDENT_CURRENT('Users_Master')
There is a lot of problems with the database as well: missing primary keys, incorrect identity columns, and missing unique constraints to name a few.

What about stored procedures? Will MSSQL server know about the same procedure being run twice or more and create a queue?

08/24/2010 23:44 abrasive#7

Quote:

Originally Posted by ProfNerwosol

What about stored procedures? Will MSSQL server know about the same procedure being run twice or more and create a queue?

In the case that you are doing a Select MAX(UserUID), no it's not going to care at all about that. It wants to finish the stored procedures execution as quickly as possibly to get on with other tasks, so it will optimize accordingly.

The best solution to adding new users and new UserUIDs is to set that column to auto-increment. Then when you add a new user, don't pass in a value for that column, and MSSQL will generate it by default when it inserts the row.

That way there's no possible way for any overlap to happen.