The most obvious mistake I see is that you let your code wide open to sql injection.
Imagine that someone give as a user: "michael';drop table users;select '".
The resulting query sent to the db would be:
Code:
select * from users where username = 'michael';drop table users;select ''
And this, will delete your table and everything in it.
When you get datas from $_GET, $_POST or $_COOKIES (and of course, $_REQUEST that merge all the three in one)
ALWAYS CHECK WHAT IS IN THERE
I use something like this:
PHP Code:
<?
/**
* Check if a string contains an sql operation keyword.
* Of it does, returns TRUE, else returns FALSE
* @param string: $par
* @return boolean
*/
function sqlInjection($par){
$ret=FALSE;
$aryRestricted =array("select","update","insert","drop","delete","truncate","alter");
foreach($aryRestricted as $k=>$v){
if(stripos($par,$v)!==FALSE){
return TRUE;
}
}
return $ret;
}
and I use it with
PHP Code:
if(isset($_GET['username'])) {
if(sqlInjection($_GET['username'])===FALSE){
$username = $_GET['username'];
}
else{
//we have an sql injection attempt, deal with it like you want to...
}
}
which can even be reduced (with the ternary operator) to
PHP Code:
$username=isset($_GET['username'] && sqlInjection($_GET['username'])===FALSE)?$_GET['username']):NULL;
if($username===NULL){
//no username given, or sql injection detected, handle it here
}
For the output part, you could simplify it too, with the <<< operator:
PHP Code:
if($row = mysql_fetch_assoc($query)) {
echo <<<html
<b>{$row['title']}</b>
<br/>
<b>{$row['username']}</b>
<br/>
<b>{$row['email']}</b>
<br/>
html;
}
It takes everything between "<<<keyword" up to "keyword;" as a string.
You can type an complete HTML block in there, rather than numerous echo.
The only things to take care of, are that you cannot call a function from inside, and that the end delimiter (html; must be on the column 0.
