php debugging help needed

[zimu]

this should be the easiest thing in the world to do but i have no clue what i'm missing. think i've got friday afternoon blues.

basically, have table "courses". in it there's an "enabled / disabled" fields.

there's a table that shows all the records and outputs whether the row is enabled or disabled. if enabled, there's a button to disable, and vice versa.

if ($row_course[enabled]=="yes")
{
print ("Enabled<br>"); ?>
<form method = post action = editcourses.php>
<input type = hidden name = id value = <? echo $row_course[id] ?>>
<input type = submit name = disableit value = "Disable">
<form> <?
}
else
{
print ("Disabled<br>"); ?>
<form method = post action = editcourses.php>
<input type = hidden name = id value = <? echo $row_course[id] ?>>
<input type = submit name = enableit value = "Enable">
<form> <?
}

up to here its fine. lets say i have two records: record 5 and 6.

for some reason when i hit enable for row 5, it enables row 6. WTF?

this is the code that does the actual modification.

if (isset($_POST[enableit])) {
print ("enabling $_POST[id]");
mysql_query("UPDATE courses SET enabled='yes' WHERE ID='$_POST[id]'") or die(mysql_error());
}
if (isset($_POST[disableit])) {
print ("disabling $_POST[id]");
mysql_query("UPDATE courses SET enabled='no' WHERE ID='$_POST[id]'") or die (mysql_error());
}

no errors. if i enable row 6 it works fine. if i enable row 5, it enables row 6. what am i missing!

 

[Crusty]

A couple of things I notice immediately that need to be fixed. When comparing strings use strcmp instead of ==

For example

if (!strcmp($row_course[enabled],"yes")) would replace if ($row_course[enabled]=="yes")

Also, on your form submission you need to sanitize your input. You are ripe for SQL injection with your current code.

 

[zimu]

hey crusty,

thanks for the response! ok, changed the strcmp. still no go though, seems to update the wrong record.

how would i sanitize my input on the form? i wasn't overly worried about that as all this code runs behind an administrator interface which doesn't even have access to the outside world...

 

[zimu]

HAHAHA i'm such a dumbass. i put <form> instead of </form> at the end of each of the sections! UGH.

would still love input as to how to sanitize it

 

[Crusty]

Basically you need to protect against someone giving you input you don't expect. There are things you can do such as escaping special characters like ' " ; to prevent the php engine from parsing those are part of the command instead of part of the data as well as checking data types.. if you are looking for a integer make sure the user actually gave you an integer and not a string or a date or whatever.

I haven't touched php in ages so I can't give more specific details but there are ways to build your SQL queries using a list of parameters that will automatically escape the characters you need.

As far as running behind an administrator interface... administrators are still users and users by nature can not be trusted.

 

[LightningRider]

Yes, you should build your sql statements with prepared statements.

Take a look at the PHP sql objects, particularly the sql statement, it will allow you to execute sql queries the way they should be.