debug SQL query

This forum is now closed as part of retiring phpBB2.
Forum rules
READ: phpBB.com Board-Wide Rules and Regulations

This forum is now closed due to phpBB2.0 being retired.
worker201
Registered User
Posts: 17
Joined: Fri Apr 11, 2008 3:49 am

debug SQL query

Post by worker201 » Sun Jun 22, 2008 9:12 am

I have an SQL query that I'm working on. Finally got it to the point where there aren't any syntax errors. Now it does absolutely nothing. I can't tell if it's my SQL query that's screwy, or my implementation.

The block of code in question:

Code: Select all

if ( isset($HTTP_GET_VARS['status']))
{
	$user_itc_status = '1';
	$invitee = $HTTP_GET_VARS['user'];
	$sql = "UPDATE " . USERS_TABLE . "
		SET user_itc_status = '1'
		WHERE username = $invitee";
	if ( !($result = $db->sql_query($sql)) )
	{
		message_die(GENERAL_ERROR, 'Could not invite user to chat', '', __LINE__, __FILE__, $sql);
	}	
}
And just to give you an idea of how my tables are organized, here's the SQL query that gets run when I manually set the value of user_itc_status for user #6 in phpMyAdmin:

Code: Select all

UPDATE `stajinaria`.`phpbb_users` SET `user_itc_status` = '1' WHERE `phpbb_users`.`user_id` =6 LIMIT 1 ;
Assistance appreciated!

User avatar
Paul
Infrastructure Team Leader
Infrastructure Team Leader
Posts: 23721
Joined: Sat Dec 04, 2004 3:44 pm
Location: The netherlands.
Name: Paul Sohier
Contact:

Re: debug SQL query

Post by Paul » Sun Jun 22, 2008 9:32 am

There need to be single quotes around the $$invitee variable. Also, you need to santinize that var to prevent SQl i njection.
Knock knock
Race condition
Who's there?

My BlogMy Photosmy phpBB Extensionscustom phpBB work & Development

worker201
Registered User
Posts: 17
Joined: Fri Apr 11, 2008 3:49 am

Re: debug SQL query

Post by worker201 » Sun Jun 22, 2008 9:46 am

Paul wrote:There need to be single quotes around the $$invitee variable.
Like this?

Code: Select all

WHERE username = '$invitee'";
Paul wrote:Also, you need to santinize that var to prevent SQl i njection.
Please explain what you mean by that. Sanitize?

User avatar
Brf
Support Team Member
Support Team Member
Posts: 50744
Joined: Tue May 10, 2005 7:47 pm
Location: {postrow.POSTER_FROM}
Contact:

Re: debug SQL query

Post by Brf » Sun Jun 22, 2008 12:27 pm

You need to use htmlspecialchars() on that input, otherwise a malicious user could put SQL statements into that string.

User avatar
Paul
Infrastructure Team Leader
Infrastructure Team Leader
Posts: 23721
Joined: Sat Dec 04, 2004 3:44 pm
Location: The netherlands.
Name: Paul Sohier
Contact:

Re: debug SQL query

Post by Paul » Sun Jun 22, 2008 12:39 pm

Brf wrote:You need to use htmlspecialchars() on that input, otherwise a malicious user could put SQL statements into that string.
uh, htmlspecialchars really dont protect against SQL injection. it only protects against XSS (In some way). You need to replace \' with '' in phpBB2 instead.
Knock knock
Race condition
Who's there?

My BlogMy Photosmy phpBB Extensionscustom phpBB work & Development

User avatar
Brf
Support Team Member
Support Team Member
Posts: 50744
Joined: Tue May 10, 2005 7:47 pm
Location: {postrow.POSTER_FROM}
Contact:

Re: debug SQL query

Post by Brf » Sun Jun 22, 2008 12:50 pm

That too. 8-)

worker201
Registered User
Posts: 17
Joined: Fri Apr 11, 2008 3:49 am

Re: debug SQL query

Post by worker201 » Mon Jun 23, 2008 3:25 am

I have made a few changes, which will help to sanitize things, but it still doesn't work.

Code: Select all

if (isset($HTTP_GET_VARS['status']))
{
	$user_itc_status = '1';
	$invitee = htmlspecialchars($HTTP_GET_VARS['user']);
        if (is_int($invitee))
        {
            $sql = "UPDATE " . USERS_TABLE . "
                    SET user_itc_status = '1'
                    WHERE user_id = '$invitee'";
            if ( !($result = $db->sql_query($sql)) )
            {
                    message_die(GENERAL_ERROR, 'Could not invite user to chat', '', __LINE__, __FILE__, $sql);
            }
        }
}
The test URL that is supposed to trigger this is yaddayadda/sample.php?status=invited&user=6

Please, if you see something wrong, spell it out to me explicitly. Thanks.

User avatar
Brf
Support Team Member
Support Team Member
Posts: 50744
Joined: Tue May 10, 2005 7:47 pm
Location: {postrow.POSTER_FROM}
Contact:

Re: debug SQL query

Post by Brf » Mon Jun 23, 2008 12:38 pm

You should use htmlspecialcharacters() with strings. You can just use intval() with integers.
You do not need to put 'quotes' around $user_id because it is a numeric value.

That said, none of those problems should prevent your code from working. Maybe you can put in some echo()s so you can see what is happening. Are you receiving any errors?

User avatar
Paul
Infrastructure Team Leader
Infrastructure Team Leader
Posts: 23721
Joined: Sat Dec 04, 2004 3:44 pm
Location: The netherlands.
Name: Paul Sohier
Contact:

Re: debug SQL query

Post by Paul » Mon Jun 23, 2008 1:58 pm

Brf wrote:You should use htmlspecialcharacters() with strings. You can just use intval() with integers.
You do not need to put 'quotes' around $user_id because it is a numeric value.

That said, none of those problems should prevent your code from working. Maybe you can put in some echo()s so you can see what is happening. Are you receiving any errors?
No, you should not use htmlspecialchars to prevent sql injection, as I said above already. Htmlspecialchars really dont prevent ql injection.
Knock knock
Race condition
Who's there?

My BlogMy Photosmy phpBB Extensionscustom phpBB work & Development

User avatar
3Di
Registered User
Posts: 11869
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milano - Frankfurt
Name: Marco
Contact:

Re: debug SQL query

Post by 3Di » Mon Jun 23, 2008 2:14 pm

This link speaks books itself I guess. http://www.phpbb.com/community/viewtopi ... 1#p1019441
Want to compensate me for my interest? Donate
Please PM me only to request paid works. Thx.
Extensions, Scripts, MOD porting, Update/Upgrades

User avatar
drathbun
Former Team Member
Posts: 12204
Joined: Thu Jun 06, 2002 3:51 pm
Location: TOPICS_TABLE
Contact:

Re: debug SQL query

Post by drathbun » Mon Jun 23, 2008 3:18 pm

Making the query secure doesn't help if the query doesn't work. :-P

Brf suggested adding some debug stuff, something like this:

Code: Select all

if (isset($HTTP_GET_VARS['status']))
{
   echo ("Got a status from GET<br />");
   $user_itc_status = '1';
   $invitee = htmlspecialchars($HTTP_GET_VARS['user']);
        if (is_int($invitee))
        {
            $sql = "UPDATE " . USERS_TABLE . "
                    SET user_itc_status = 1
                    WHERE user_id = $invitee";
            echo ("going to execute $sql<br />");
            if ( !($result = $db->sql_query($sql)) )
            {
                    message_die(GENERAL_ERROR, 'Could not invite user to chat', '', __LINE__, __FILE__, $sql);
            }
            else
            {
                    echo ("Executed $sql, affected " . $db->sql_numrows($result));
            }
        }
}
That will give you some idea of what's going on, which should help with the debugging. Also I took out a few quotes; as mentioned, you don't need quotes for the user_id value, and assuming that user_itc_status is a number field you don't need quotes for the '1' value either.

Once you get the simple version work, then you can go back and worry more about getting it secured.
I blog about phpBB: phpBBDoctor blog
Still using phpbb2? So am I! Click below for details
Image

User avatar
3Di
Registered User
Posts: 11869
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milano - Frankfurt
Name: Marco
Contact:

Re: debug SQL query

Post by 3Di » Mon Jun 23, 2008 3:33 pm

Yes, that's normal school. Forgot to mention. :P
Want to compensate me for my interest? Donate
Please PM me only to request paid works. Thx.
Extensions, Scripts, MOD porting, Update/Upgrades

worker201
Registered User
Posts: 17
Joined: Fri Apr 11, 2008 3:49 am

Re: debug SQL query

Post by worker201 » Mon Jun 23, 2008 11:33 pm

After running the following code:

Code: Select all

if (isset($HTTP_GET_VARS['status']))
{
	echo ("Got a status from GET<br />");
	$user_itc_status = 1;
	$invitee = intval($HTTP_GET_VARS['user']);
        if (is_int($invitee))
        {
            $sql = "UPDATE " . USERS_TABLE . "
                    SET user_itc_status = 1
                    WHERE user_id = $invitee";
            echo ("going to execute $sql<br />");
            if ( !($result = $db->sql_query($sql)) )
            {
                    message_die(GENERAL_ERROR, 'Could not invite user to chat', '', __LINE__, __FILE__, $sql);
            }
            else
            {
                    echo ("Executed $sql, affected " . $db->sql_numrows($result));
            }
        }
}
I get the following output:
Got a status from GET
going to execute UPDATE phpbb_users SET user_itc_status = 1 WHERE user_id = 6

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/.tech/stajftp/stajinaria.net/forum/db/mysql4.php on line 167
Executed UPDATE phpbb_users SET user_itc_status = 1 WHERE user_id = 6, affected
But it did in fact work, as verified by checking the tables in phpMyAdmin.

I'm going to read up on that link about securing mods, and will probably be modifying the code later.
FYI, user_itc_status in the table is a tinyint.

User avatar
Brf
Support Team Member
Support Team Member
Posts: 50744
Joined: Tue May 10, 2005 7:47 pm
Location: {postrow.POSTER_FROM}
Contact:

Re: debug SQL query

Post by Brf » Mon Jun 23, 2008 11:35 pm

Yes. It said it worked.
Why did you think it didnt before?

worker201
Registered User
Posts: 17
Joined: Fri Apr 11, 2008 3:49 am

Re: debug SQL query

Post by worker201 » Tue Jun 24, 2008 2:32 am

There's another place on the page that changes a piece of text based on the value of user_itc_status. When that didn't change, I checked the tables, to see if it was the query failing or the text change. The tables indicated that the query was not executing properly, which is why I came here for some help. It seems clear to me now that the syntax of the query, specifically this line:

Code: Select all

SET user_itc_status = '1'
was the cause of the problem. My guess is that putting single-quotes around the value makes it a string, which didn't mesh with the tinyint constraint of the table. Am I close?

edit - my understanding is that SQL injection only applies to string values. The only value I am inserting into the table is an integer, and there are two checks to make sure that user_id is an integer as well. So am I secure? My page contains the following, as per general suggestion:

Code: Select all

define('IN_PHPBB', true);
$phpbb_root_path = './';
include($phpbb_root_path . 'extension.inc');
include($phpbb_root_path . 'common.'.$phpEx);
Last edited by worker201 on Tue Jun 24, 2008 2:46 am, edited 1 time in total.

Post Reply

Return to “[2.0.x] MOD Writers Discussion”

Who is online

Users browsing this forum: No registered users and 5 guests