Query error using variables (I think)

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
mcccy005
Forum Contributor
Posts: 123
Joined: Sun May 28, 2006 7:08 pm

Query error using variables (I think)

Post by mcccy005 »

Basically I have an input form, and the form validates correctly when the submit button is pressed and succesfully redirects the user to the next page. The problem is the data isn't actually going into the database but there's no error being displayed anywhere.
The code I am using works fine on another page except the only difference is the query so I'm thinking that must be the problem so can anyone see any problems with this query??
(I'm THINKING the error might be with the way I implement the $region_id and $secure_parking variables which are defined higher up in the php file that this query is in, but I've tried different variations and can't get it to work at all.)
Either help with the query would be good, or otherwise some way to test why data isn't going into the database but no error is being displayed would be great. Thanks in advance.

Code: Select all

query = "INSERT into marinas (marina_name, abn, region_id, contact_first_name, contact_last_name, street_address,
		     suburb, state, postcode, postal_address, postal_state, postal_postcode, telephone_work, telephone_fax, 
                                     telephone_mobile, telephone_emergency, email, secure_parking) 
		     VALUES ('{$_POST["marina_name"]}',
			'{$_POST["abn"]}',
			'$region_id',
			'{$_POST["contact_first_name"]}',
			'{$_POST["contact_last_name"]}',
			'{$_POST["street_address"]}',
			'{$_POST["suburb"]}',
			'{$_POST["state"]}',
			'{$_POST["postcode"]}',
			'{$_POST["postal_address"]}',
			'{$_POST["postal_state"]}',
			'{$_POST["postal_postcode"]}',
			'{$_POST["telephone_work"]}',
			'{$_POST["telephone_fax"]}',
			'{$_POST["telephone_mobile"]}',
			'{$_POST["telephone_emergency"]}',
			'{$_POST["email"]}',
			'$secure_parking')";
User avatar
jayshields
DevNet Resident
Posts: 1912
Joined: Mon Aug 22, 2005 12:11 pm
Location: Leeds/Manchester, England

Post by jayshields »

- No $ preceeding query variable
- No execution of query
- Use or die("Cannot execute query ".mysql_error()."<br />".$query); after query execution
- Don't put $_POST variables straight into queries.
- Don't use the quotes inside quotes without escaping them
mcccy005
Forum Contributor
Posts: 123
Joined: Sun May 28, 2006 7:08 pm

Post by mcccy005 »

With regard to the $ before query, that was my error copying the code over.
There is an execution of the query in another script (as it's sent to another file as I'm using a bunch of objects I created to create a series of related input forms and these all work fine for another page using the same objects);
WIll have a look into the die thingy you're talking about;
What's wrong with putting $_POSt straight into queries?? (Like I said it works fine with another page which is what I also use; and I see it as a waste of memory storing each $_POST variable into a separate variable just to throw it straight into a query. Seems like duplication to me, but maybe theres something I don't know about???;
and I have no idea what you mean by escaping a quote inside a quote sorry???

Many thanks again,
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

mcccy005 wrote:With regard to the $ before query, that was my error copying the code over.
There is an execution of the query in another script (as it's sent to another file as I'm using a bunch of objects I created to create a series of related input forms and these all work fine for another page using the same objects);
WIll have a look into the die thingy you're talking about;
What's wrong with putting $_POSt straight into queries?? (Like I said it works fine with another page which is what I also use; and I see it as a waste of memory storing each $_POST variable into a separate variable just to throw it straight into a query. Seems like duplication to me, but maybe theres something I don't know about???;
and I have no idea what you mean by escaping a quote inside a quote sorry???

Many thanks again,
Various issues here. You're trying to literally place $_POST["foo"] into a double quoted string so the "foo" part is causing a syntax error in PHP itself. You need to concatenate, use single quotes for the 'foo' bit, or just lose the quotes entirely since you're in string context.

> What's wrong with putting $_POSt straight into queries??

Bad, very bad. Look up "SQL Injection attacks". By allowing user-submitted data into queries you're just going to run against MySQL you give an attacker the opportunity to run any query they like in your database! You also run he risk of getting syntax errors in the query itself if you have quotes, or invalid characters in there.

mysql_real_escape_string() helps keep you safe ;)
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Post by d3ad1ysp0rk »

Just by looking at the syntax highlighted code, it's quite easy to see what's wrong.

change

Code: Select all

VALUES ('{$_POST["marina_name"]}',
to

Code: Select all

VALUES ('" . $_POST["marina_name"] . "',
and so forth.

But you really should change those post variables out, for security reasons.
staar2
Forum Commoner
Posts: 83
Joined: Fri Apr 06, 2007 2:57 am

Post by staar2 »

i could write in one field 'drop table marinas;. And you should check data before sending it to database. Use at least mysql_escape_string(). :!:
staar2
Forum Commoner
Posts: 83
Joined: Fri Apr 06, 2007 2:57 am

Post by staar2 »

//edit oops almost same post like others. Sorry didn't read them.
User avatar
jayshields
DevNet Resident
Posts: 1912
Joined: Mon Aug 22, 2005 12:11 pm
Location: Leeds/Manchester, England

Post by jayshields »

I think dropping the table would be a minor worry, aslong as back ups are being made - what benefit would the attacker get from dropping a table anyway?
mcccy005
Forum Contributor
Posts: 123
Joined: Sun May 28, 2006 7:08 pm

Post by mcccy005 »

Ok; so here's the code from the file that DOES work:

Code: Select all

$db_connection=houseboat_db_connect( );
$region_info_query = "INSERT into regions (region_name, state)
		VALUES ('{$_POST["region_name"]}',
		'{$_POST["state"]}')";
However, when I now change it to the following:

Code: Select all

$db_connection=houseboat_db_connect( );

	$region_name=mysql_real_escape_string($_POST["region_name"], $db_connection); //Line 9
	$state=mysql_real_escape_string($_POST["state"]); //Line 10

	$region_info_query = "INSERT into regions (region_name, state)
			VALUES ('$region_name',
			'$state')";
I get the following errors:
mysql_real_escape_string() expects parameter 2 to be resource, object given in X:\XXX.php on line 9;
and
mysql_real_escape_string() [function.mysql-real-escape-string]: Access denied for user 'ODBC'@'localhost' (using password: NO)
as well as mysql_real_escape_string() [function.mysql-real-escape-string]: A link to the server could not be established in X:\XXX.php, both on line 10. (I am aware one line is using the above set $db_connection, and the other is not (ie. it's using the last set connection which should be $db_connection).

This is how I set $db_connection in a globally accessible folder:

Code: Select all

function houseboat_db_connect( )
{
	$host_name = 'localhost'; //host name to connect to mysql server
	$username = 'root'; //username to connect to mysql server
	$password = 'XXXX'; //password to connect to mysql server
	$database_name = 'houseboat_db'; //name of database to connect to in mysql
	
	/*Connects to houseboat database on mysql server using the following
	arguments: (host_name, mysqli_username, mysql_password, mysql_database_name)*/
	$db_connection = new mysqli($host_name, $username, $password, $database_name);
	

	/*Returns error if unable to connect to the above database*/
	if(mysqli_connect_errno( ))
	{
		new error("Unable to connect to database. Error number: ".mysqli_connect_error());
		exit;
	}
			
	/*Returns the object which stores the database connection*/
	return $db_connection;
}
(And yep I know I shouldn't be using the root user, but its purely for testing purposes at this stage).

This function works fine in every other file I access it from.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

You are connecting with mysqli, so you need to use the mysqli escape function mysqli_real_escape_string().
mcccy005
Forum Contributor
Posts: 123
Joined: Sun May 28, 2006 7:08 pm

Post by mcccy005 »

Awesome; thanks. That seems to work.
I just wanted to verify that the following code sql injection safe as well as ok in regard to the quotes etc. mentioned earlier in the post?? (It works fine):

Code: Select all

$region_info_query = "INSERT into regions (region_name, state)
		     VALUES ('".$db_connection->real_escape_string($_POST['region_name'])."', '".
		   	   $db_connection->real_escape_string($_POST['state'])."')";
Many thanks again in advance.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

It is safer. But do you really want to take something directly from a user input and put into your database without knowing what it is? Seriously... what if you are execting a string and the user supplies a number. What if you are expecting only alpha numeric values and the user supplies special characters? There should be something in place to catch that.
mcccy005
Forum Contributor
Posts: 123
Joined: Sun May 28, 2006 7:08 pm

Post by mcccy005 »

Yeah I do actually do that.

My code is structured as follows:
(from the top to the bottom)

* input_set (contains an array of input_pages and other stuff)
* input_page (contains an array of input_fields), extends input_set
* input_field (contains a data-type to be stored in this field (eg. numeric, date, money, address, email, ABN, password etc.), extends input_page
* text_field, password_field, radio_field, etc., extends input_field

When each page first loads, I call a validate( ) function which checks whether the field (ie. $_POST['field_name'] has to have data in it; and if the field does have data in it (regardless of whether it has to or not), the field will still have to have a data_type associated with it (eg. telephone number, email etc.), I then validate that it is in fact the appropriate data_type. If it is not, then the invalid fields will be highlighted and no data will be put into the database.
Post Reply