[Release] processor.php protection for potential security risk

[il.mane]

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

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;
}

[ProfNerwosol]

Vulnerable to code injections in what way?

[abrasive]

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:

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:

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.

[jamessimpler]

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

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:

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:

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.

[abrasive]

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.

[ProfNerwosol]

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:

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:

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?

[abrasive]

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.