...

View Full Version : Check if this is secure?



Blatch
09-08-2009, 07:04 AM
$url = htmlspecialchars($_GET["url"]);

$url_exist = mysql_query("SELECT * FROM `websites` WHERE `url` = '$url' AND `status` = 1 ORDER BY `id`") or die(mysql_error());

if(!isset($url)) {
header("Location: http://www.mysite.com/");
} else {
if($_GET["url"] == $url && mysql_num_rows($url_exist) > 0) {
mysql_query("UPDATE `websites` SET `out` = `out` + 1 WHERE `url` = '$url'");
header("Location: " . $url);
} else {
header("Location: http://www.mysite.com/");
}
}

Is this safe or are there flaws? Please help.

abduraooft
09-08-2009, 07:11 AM
The translations performed are:

* '&' (ampersand) becomes '&'
* '"' (double quote) becomes '"' when ENT_NOQUOTES is not set.
* ''' (single quote) becomes ''' only when ENT_QUOTES is set.
* '<' (less than) becomes '&lt;'
* '>' (greater than) becomes '&gt;'

Better to use mysql_real_escape_string() (http://php.net/mysql_real_escape_string) (after removing the slashes added by magic_quote_gpc, if any)

PS:
header("Location: " . $url); Don't you need to validate the domain name?

Blatch
09-08-2009, 07:26 AM
Ok changed it up.

You mean check if the domain is a real domain? I already did that in my submission form (checks to see if url of site is real and in existence) so I didn't think I would have to do it again. And if the site isn't in my database, it'll just redirect you to mysite instead of the url.

Anything else I should do?

abduraooft
09-08-2009, 07:34 AM
I already did that in my submission form (checks to see if url of site is real and in existence) so I didn't think I would have to do it again. Before submission? That may not be enough. You'd need to validate all external data from server side, to save your tables from wrong data.

Blatch
09-08-2009, 07:42 AM
But it will only update the table data if the website is in existence in the database, if not, it won't do anything. So it's pointless for the user to put in a random website in the url param. Right?

abduraooft
09-08-2009, 08:03 AM
So it's pointless for the user to put in a random website in the url param. Right? Yes. That's OK. I thought there's an INSERT query :)

Fou-Lu
09-08-2009, 01:14 PM
Although it won't make a difference in this situation (because of you're if/else usage), header redirect should be followed by an exit(). This is because PHP will continue to process regardless of if a browser has been redirected (it has to wait until the end anyway when it receives its results).
Its a good habit to get into, and I'd recommend even changing what you have to reflect this. abduraooft covered the rest from the looks of it.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum