Secure PHP Query?

Question

I have a page to activate users who click the activation link in an email I send them, the email contains a url which posts there email address using the GET method, My code is as follows but what im asking is this a secure method or is there any way it can be imporved? Thanks

$username = $_GET['email']; 

mysql_query("UPDATE users SET active = 'yes'
WHERE email = '$username'") or die('oops!');

echo "<meta http-equiv=\"refresh\" content=\"0;URL=/index.php?msg=active\">";

Just call me [Bobby Tables](http://xkcd.com/327/). Or use [PDO](http://stackoverflow.com/questions/6379433/mysql-prepared-statements). — Matt Ball, Jun 17 '11 at 17:49
Also check out prepared statements: http://dev.mysql.com/tech-resources/articles/4.1/prepared-statements.html — Phil, Jun 17 '11 at 18:07
potential for ultimate destruction there. at least use mysql_real_escape_string. I was about to type that as an answer, but a billion people already said the same thing. always validate your input before sending it to the database. — stephenbayer, Jun 17 '11 at 18:08
I don't have enough downvotes to downvote all the answers that didn't mention PDO or prepared statements, or to upvote all the ones that did. — Lotus Notes, Jun 17 '11 at 18:17

score 5 · Answer 1 · answered Jun 17 '11 at 17:50

This isn't the best method for activating an email. Anyone could register with a fake email and create their own GET request to activate it. You should use a unique hash instead that cannot be guessed. Also, you need to escape your SQL statement with mysql_real_escape_string.

score 5 · Answer 2 · edited May 23 '17 at 12:09

5

Forget about mysql_real_escape_string — see https://stackoverflow.com/questions/5690795/am-i-safe-using-just-mysql-real-escape-string-to-defend-sql-injections/5690877#5690877 and Do htmlspecialchars and mysql_real_escape_string keep my PHP code safe from injection?.

Use PDO.

Don't forget about second-order SQL injection (what, no one told you that security is hard?). More info here.

edited May 23 '17 at 12:09

Community

1
1

answered Jun 17 '11 at 17:51

Matt Ball

354,903
100
647
710

Thank, may I ask why I should forget mysql_real_escape_string? – Liam Jun 17 '11 at 17:54
@Liam see my edit. Basically, you have to **always** remember to escape **everything.** – Matt Ball Jun 17 '11 at 17:59
Thanks a lot @matt, appreciate it! – Liam Jun 17 '11 at 18:02
This is the only correct answer so far other than @Michael's. – Lotus Notes Jun 17 '11 at 18:17
@Lotus more than one Michael has posted an answer – Matt Ball Jun 17 '11 at 18:18

George Cummins · Accepted Answer · 2011-06-17T17:56:47.767

Consider the following value of $_GET['email']:

"'; DELETE FROM users WHERE 1 or username = ''"

If your user submits this value, your script will execute the following SQL statement:

UPDATE users SET active = 'yes' WHERE email = ''; DELETE FROM users WHERE 1 or username = ''"

You will need to filter your input to remove characters that have meaning in an SQL setting (\' in this example).

You can help secure your queries by using PDO prepared statements, or by filtering using mysql_real_escape_string().

Additionally, you want to create a unique, non-guessable value to send to the user, and expect that value back from $_GET. If you allow the user to supply a plain-text email address, a malicious user can activate any email address. md5() or other hash functions can be used to create unique values.

Although the some ideas of this post are correct, mysql cannot actually execute more than one statement in a single call to `mysql_query()`. SQL injection is still a vulnerability, but an attacker won't actually be able to delete all users from an update statement. — Lotus Notes, Jun 17 '11 at 18:11

score 3 · Answer 4 · answered Jun 17 '11 at 17:48

3

This is not at all secure. You need to call mysql_real_escape_string() on your $username

$username = mysql_real_escape_string($_GET['email']);

Better would be to use PDO and pass the variable in through a prepared statement.

answered Jun 17 '11 at 17:48

Michael Berkowski

267,341
46
444
390

score 2 · Answer 5 · answered Jun 17 '11 at 17:48

2

No, that's vulnerable to SQL injection. I can place a ' in $_GET['email'] and run any query I like.

answered Jun 17 '11 at 17:48

ceejayoz

176,543
40
303
368

score 2 · Answer 6 · answered Jun 17 '11 at 17:49

2

This is most definitely not secure. You need to escape any potentially dangerous characters (think if someone entered a ' in their email address) via mysql_real_escape_string.

answered Jun 17 '11 at 17:49

Jeff Hubbard

9,822
3
30
28

score 2 · Answer 7 · answered Jun 17 '11 at 17:49

2

It is not secure. You should either use stored procedures, or escape the sql.

answered Jun 17 '11 at 17:49

Michael Lowman

3,000
1
20
34

score 2 · Answer 8 · answered Jun 17 '11 at 17:52

Imagine for a moment, that I want to do something malicious to you site, now suppose that I manipulate your URL and change .php?email=arron@arronchapman.com to .php?email=1';SHOW TABLES or maybe even .php?email=1';DROP TABLE USERS; that wouldn't be very nice would it?

The simplest way to prevent this is to wrap all of your input values in the function mysql_real_escape_string, this would make your query look like this: mysql_query("UPDATE users SET active = 'yes' WHERE email = '" . mysql_real_escape_string($username) .'") or die('oops!');

I'd like to introduce you to my friend Bobby Tables (From XKCD): enter image description here

Bill B · Answer 9 · 2011-06-17T18:16:11.020

Aside from the SQL injection issues that everyone else mentioned, you have the problem of people being able to forge their own activation link and bypass the need for receiving the email.

A common solution is to include some sort of hash value in the URL that you check before activating.

For example -

Create a table for activations. When the user signs up, insert the email address (or some other unique identifier) and some non-constant, secret value. This could be a random number, a timestamp and a salt, or similar.

When you generate the link for the email, include a hash value obtained from hashing both the email address and the secret value.

Then when the user clicks on the link, you can take the email address from the URL, hash it with the value in the database, and see if it matches the hash in the URL.

This will make it much harder for someone to forge an activation link.

score 0 · Answer 10 · answered Jun 17 '11 at 17:59

While, as everyone is saying, it is an absolute necessity to escape your string before using it in a query, I think there is more be concerned with.

This still allows non users to try different email addresses and see what the result will be.

I recommend encrypting the email, and using the encrypted version of the email in the url.

So when you send them the link:

$enc_key = 'SOME KEY I USE';
$email = mysql_real_escape_string($email);
$result = $db->query("SELECT AES_ENCRYPT('$email', '$enc_key') as email");
$enc_email = mysql_fetch_assoc($result);
$enc_email = $enc_email['email'];
$url = "http://www.example.com/confirm.html?key=$enc_email";

then to check for the user on confirm you:

$enc_key = 'SOME KEY I USE';
$enc_email = mysql_real_escape_string($_GET['key']);
$result = $db->query("UPDATE users SET active = 1 WHERE AES_ENCRYPT(email, '$enc_key') = '$enc_email'");

Secure PHP Query?

10 Answers10