SQL Injection issue. (Passed email validation?)

Question

I was recently been hacked and hackers retrieved all my mailing_list data and I wondered how did they passed the "Validate_email" function and managed to do sql injection?

this is my code:

function validate_email($address)
{
            return (ereg('^[-!#$%&\'*+\\./0-9=?A-Z^_`a-z{|}~]+'. '@'. '[-!#$%&\'*+\\/0-9=?A-Z^_`a-z{|}~]+\.' . '[-!#$%&\'*+\\./0-9=?A-Z^_`a-z
{|}~]+$', $address));
}

if (empty($_REQUEST['email']) || !validate_email($_REQUEST['email'])) {
    die('INVALID EMAIL');
}

mysql_query("
    REPLACE INTO mailing_list
    SET email='".strtolower($_REQUEST['email'])."'
") or die('Unable to insert email to database');

die(header('Location: http://www.***'));

I was wondering how they did it with the validate email function.. and if this is the place they managed to hacked really or i'm just mistaking and I need to search more..

Just don't use `mysql_query`. [**At all**](http://www.php.net/manual/en/faq.databases.php#faq.databases.mysql.deprecated). Use prepared statements instead. — Oliver Charlesworth, Apr 07 '13 at 13:46
I know its a really old code.. I was just wondering how they hacked this one if i'm using preg_replace, what string did they use? and to see if it's really here where they managed stole all of my database data. — D_R, Apr 07 '13 at 13:52
How you are too sure they stole data by exploiting this function? It seems you have more security hole in your application. If you wish you can give us access to your site to testing. Normally if the validate email function do not create a backdoor, they can't stole data with it. I can garantee you. Thanks — , Apr 07 '13 at 15:15

score 4 · Answer 1 · answered Apr 07 '13 at 13:46

4

You are allowing ' in e-mail addresses. While this might be valid in an e-mail address, it is not a good idea to allow it in the mysql_query. Replace the query with this:

mysql_query("
   REPLACE INTO mailing_list
   SET email='".mysql_real_escape_string(strtolower($_REQUEST['email']))."'
   ") or die('Unable to insert email to database');

For the future, consider switching to prepared statements, as these are much safer regarding SQL injections.

answered Apr 07 '13 at 13:46

mrks

8,033
1
33
62

2

You can write the same secure/insecure statements with both `mysql_query`+`mysql_real_escape_string` and prepared statements. The key is to know *why* SQL injections occur. – Gumbo Apr 07 '13 at 13:52
@Gumbo: The key is really to *always use bound parameters*. Then SQL injection becomes an irrelevancy. – Oliver Charlesworth Apr 07 '13 at 13:54
True. The bad news is that there are no libraries (beside my own) that let's you *always* use bound parameters. – Your Common Sense Apr 07 '13 at 14:00
1

_always use bound parameters_: that may not _always_ be possible - for example, if one needs table names or, more commonly, column names as input (http://stackoverflow.com/a/8314141/347934). – MvanGeest Apr 07 '13 at 14:07
@MvanGeest: Sure, but using user input directly as a table name isn't a good idea anyway. (Perhaps a better way to phrase this is "never construct queries by concatenating with user input"...) – Oliver Charlesworth Apr 07 '13 at 14:10
@MvanGeest That’s the exact reason why it’s more important to understand why SQL injections happen. User input is not always data but may sometimes also be used as part of the command. “Always use bound parameters” won’t work here. – Gumbo Apr 07 '13 at 14:20
@Gumbo: Indeed, the purpose of my comment was to point out that a strategy containing _always_ will usually fail in some of the individual cases, or is not applicable to some of them. In general, I think gaining knowledge about the cause of a problem is more likely to help someone to produce solid systems than applying one solution strategy. – MvanGeest Apr 07 '13 at 14:30
@MvanGeest: Fair point. But note that there are multiple reasons to avoid using the `mysql_***` functions. And also that there are certain strategies (which bound parameters form a part of) that, if followed consistently, avoid having to worry about SQL injections ever again... – Oliver Charlesworth Apr 07 '13 at 15:45
@OliCharlesworth: That certainly sounds possible. Is there a "canonical" question/answer discussing those here on StackOverflow that we could link to? – MvanGeest Apr 07 '13 at 16:59
@MvanGeest I edited the answer you linked to in order to show a strategy for allowing semi-dynamic column names and table names. – Milo LaMar Jul 19 '14 at 17:21

score -1 · Answer 2 · answered Apr 07 '13 at 13:47

-1

its because you did not used mysql_real_escape_string

instead of directly going to use the $_REQUEST['email']

$email = mysql_real_escape_string($_REQUEST['email']);
mysql_query("
REPLACE INTO mailing_list
SET email='".strtolower($email)."'
") or die('Unable to insert email to database');

answered Apr 07 '13 at 13:47

NosiaD

587
1
6
18

mysql_real_escape_string is not a magical wand makes your data safe, but the phrasing implies that – Your Common Sense Apr 07 '13 at 13:49

score -1 · Answer 3 · answered Apr 07 '13 at 13:59

-1

something like entering the following as an email address...

a@a.com';select * from mailing_list;

answered Apr 07 '13 at 13:59

Randy

16,480
1
37
55

3

Oh please, please try it first yourself – Your Common Sense Apr 07 '13 at 14:01
1

This is the only one that actually answers the question. – Ash Burlaczenko Apr 07 '13 at 14:03
@Randy Already tried that, didn't work still getting Invalid Email. – D_R Apr 07 '13 at 14:05
well - something like that - maybe it is the * - hard to parse the regex in my head :) – Randy Apr 07 '13 at 14:07
@Randy yeah I know I was trying to figure it out myself how they managed to do it. and if it's from this file. anyway even when I'm switching the * with "email" still getting invalid email. – D_R Apr 07 '13 at 14:10
@DanRevah don't bother. it won't work anyway, even if you remove your validation at all. – Your Common Sense Apr 07 '13 at 14:10
@YourCommonSense got any other ideas? – D_R Apr 07 '13 at 14:11
1

`mysql_query` doesn’t allow multiple statements. – Gumbo Apr 07 '13 at 14:21
2

even if it was, just select to nowhere will no good to attacker anyway - there is no output either – Your Common Sense Apr 07 '13 at 14:22

score -2 · Answer 4 · edited Jun 20 '20 at 09:12

-2

A Golden Rule Of Protection:

Data validation should never be used to substitute SQL data formatting.

Your query have to be properly formatted despite of whatever validations that has been (or has not been) performed.

if this is the place they managed to hacked really or i'm just mistaking and I need to search more..

Nevertheless, this query is scarcely could be the cause of stolen emails. So, you have to watch somewhere else. Judging by this code, there are many other weak spots.

edited Jun 20 '20 at 09:12

Community

1
1

answered Apr 07 '13 at 13:52

Your Common Sense

156,878
40
214
345

I can't see an answer to his question here. – Ash Burlaczenko Apr 07 '13 at 14:02

SQL Injection issue. (Passed email validation?)

4 Answers4

Data validation should never be used to substitute SQL data formatting.