Code dissappeared when upgraded to PHP5! Help!

[ade1982]

Hi All,

This is me probably not coding it properly, but I have a problem

the page that passes the variable is:

<?php echo "<a href=\"casestudy.php?id=" . urlencode($info3[name]) . "\"> " . $info3[name] . " </a>";?> 

where $info3[name] is a varchar

and the page that uses it is

casestudy.php

if (!$id) {
	header("Location: focus.php");
}

$id = urldecode($id);

$sql = mysql_query("SELECT * FROM focus WHERE name='$id'");

while($info3 = mysql_fetch_array( $sql )) {
	$name = $info3[name];
	$company = $info3[company];
}

Now this worked until Friday when the server was upgraded to PHP5, whereas before it was running PHP4.

Anyone know what is wrong?!

Thanks in advance

Ade

[bradgrafelman]

ade1982 wrote:

Anyone know what is wrong?!

Yep - you never defined $id. register_globals, a directive your script depended on, has long since been declared a security vulnerability and thus deprecated - it's not even available anymore as of PHP6.

What you need to do is properly declare your variables that reference external data by using the superglobal arrays (more info here: [man]variables.superglobals[/man]). In other words, $id should be defined using $GET['id'] ($GET since it's sent via the GET method, e.g. in the query string).

Also note that code such as if(!$_GET['id']) causes notices to be issued if the id isn't set. What you should instead be doing is using [man]isset/man or [man]empty/man to check if a variable exists/is empty.

Finally, your code appears to be vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query string. Instead, it should first be sanitized with a function such as [man]mysql_real_escape_string/man. In your case, since you're expecting an integer (an ID, I'm assuming, is an integer), you could cast the data to an int:

$id = (int)$_GET['id'];

before using it in the SQL query.