forgot password script injected

[yarray]

Can someone help me out with this bit of code. Today when i tryed to onto the administrators side of my site i was greeted with not being abke to log in because my password had been changed. Now i have gotten 33 emails in reguard to this on march 3, meaning 33 requests for email change from my server for my acount. Like i said i just tryed to log in and the hashed password was different, so obviously someone changed something somehow, someone redirected the password recovery to their email acount, or they could have even bypassed that im not sure, all i know is i need to get this fixed.

if (isset ($_POST['forgot']))
  {
      $backslash = '\\';
    $invalid = array ('http', ':', '/', '//', '?', '<', '>', ('' . '$'), '{', '}', '\'', '&', '%', '#', '\\');
      $email = $_POST['email'];
    $email = str_replace ($invalid, '', $email);
    $email = mysql_real_escape_string($email);
    $result = mysql_query ('' . 'SELECT username, email from users WHERE email=\'' . $email . '\'');
    $found = mysql_num_rows ($result);
    if ($found < 1)
    {
      echo '<div class="error">Email not found in database ' . $email . '<img src="images/error.gif"></div>';
    }
    else
    {
      $worked = mysql_fetch_array ($result);
      $username = $worked['username'];
      $random = unique_id (25);
      mysql_query ('' . 'INSERT INTO lostpassword VALUES (\'' . $random . '\', \'' . $username . '\')');
      $message = '' . 'Someone has requested a lost password request for your account at ' . $sitename . '.To set a new password, please go to ' . $siteurl . '/?p=forgot&code=' . $random;
      mail ($email, '' . 'Lost Password at ' . $sitename, $message, ((('' . 'From: ' . $emailaddress . '') . 'Reply-To: ' . $emailaddress . '') . ''));
      echo '<div class="error">New password request email sent <img src="images/complete.gif"></div>';
    }
  } 

[PeterB89]

Is it possible for the users to signup with an username what is already taken?
You should check your $result query with the email AND the username which is filled in.

Also add LIMIT 1 to your query, cause you know you will get only one result.

I see you don't change the password directly, the password won't get changed after a person gets to => ?p=forgot&code=' . $random

Could you show us that bit of code, maybe the problem lies there?

[superuser2]

I see you don't change the password directly, the password won't get changed after a person gets to => ?p=forgot&code=' . $random

He is changing the password and sending the mail before it's confirmed the account holder actually wanted to change their password. You're describing what he should be doing, not what he is doing.

yarray,

That's what you'll need to do. The password should not be touched until the owner of the email address on file clicks a link in their email. Nothing should be touched until it's confirmed that someone w/ access to the email address wants it to be.

[yarray]

Nothing is being touched untill the user actually clicks on the verification link that is set via email.

The below code is what happens after the user clicks on the link in the email. He is then directed to a page with a form that asks for new password.

if (isset ($_POST['forgotnew']))
  {
    $password = htmlspecialchars ($_POST['password']);
    $password2 = htmlspecialchars ($_POST['password2']);
    if ((empty ($password) OR empty ($password2)))
    {
      echo '<div class="error">Password cannot be blank <img src="images/error.gif"></div>';
    }
    else
    {
      if ($password !== $password2)
      {
        echo '<div class="error">Passwords must match <img src="images/error.gif"></div>';
      }
      else
      {
        $code = htmlspecialchars ($_GET['code']);
        $result = mysql_query ('' . 'SELECT username FROM lostpassword WHERE code=\'' . $code . '\'');
        $worked = mysql_fetch_array ($result);
        $username = $worked['username'];
        $password = md5 ($password);
        $result = mysql_query ('' . 'UPDATE users SET password=\'' . $password . '\' WHERE username=\'' . $username . '\'');
        mysql_query ('' . 'DELETE FROM lostpassword WHERE username=\'' . $username . '\'');
        echo '<div class="error">New password set <img src="images/complete.gif"></div>';
      }
    }
  } 

ATM i have no idea how my password was changed. I can post more code at your request and thanks for the help cause i need to fix this asap.

[lorenw]

It sounds like someone tried something like this

http://www.unixwiz.net/techtips/sql-injection.html

You should be ok if you use mysql_real_escape_string.

I use this irregardless of the magic quotes setting

function sanitize_me($arr) 
{
    foreach ($arr as $k => $v) 
    { 
        if (is_array($v)) 
            { 
            $arr[$k] = preg_replace("[\x5c\]","",$v);
//            $arr[$k] = strip_magic_quotes($v); 
            $arr[$k] = htmlentities("$arr[$k]",ENT_QUOTES);
            $arr[$k] = mysql_real_escape_string($arr[$k]); 
//            echo"<br>this is sanitize_me arr $arr[$k]<br>";
            } 
        else 
            { 
            $arr[$k] = preg_replace("[\x5c\]","",$v); //this removes all \s , also magic quotes is turned off but why allow \s from $GET.
//            $arr[$k] = stripslashes($v); // is this better than preg replace?
            $arr[$k] = htmlentities("$arr[$k]",ENT_QUOTES); // convert the special Chars.
            $arr[$k] = mysql_real_escape_string($arr[$k]);  // this should never happen but just in case.
//            echo"<br>this is sanitize_me string $arr[$k]<br>";
             } 
    } 
    return $arr; 
} 
    if (!empty($_GET))    { $_GET =  sanitize_me($_GET);    } 
    if (!empty($_POST))   { $_POST =  sanitize_me($_POST);   } 
    if (!empty($_COOKIE)) { $_COOKIE = sanitize_me($_COOKIE); } 

hth
Loren

[PeterB89]

I see you're creating a random code when an user requests his password. At the "forgetnew" page, you get the username just by selecting the username with that random code.

I don't think that's real safe. However it's (almost) impossible to guess that number and creating an random code which is exactly the same as another random code you created before is as impossible either. But there still is a chance!
I think you should create this whole password recovery system over, cause it is really necessary that this works 100% safe.

What I think your problem is right know is, what lorenw said before, an SQL injection.
What if some user typed in something like this:

?code=1 OR username=admin

The above example doesn't work, but I'm sure there is an injection for this one.


Your solution:
Use mysql_real_escape_string as lorenw said. I see the random code you generate is an unique_id(), so I guess this are numbers only? Then you should check the url input $_GET['code'] with ctype_digit(), so only numbers would be accepted.