Yet Another Way You Shouldn’t Implement Password Recovery

A couple of weeks ago I had the dubious pleasure of calling a company to tell them that they had a security vulnerability in their site that you could drive a truck through. I’m not going to name names, but given that the issue is now patched, I think it’s a good lesson on things to avoid when designing your own application.

I’ll show you the relevant source code first and let you see if you can figure it out.

// forgot.php ("forgot password" functionality)

if(!empty($_POST['username'])) {
    // send the request
    CURLHandler::Post(LOGIN_APP_URL . 'resettoken', array('username' => $_POST['username']));
    $result = ob_get_contents();

    $result = json_decode($result);
    if ($result->success == true) {
        $resetUrl = SECURE_SERVER_URL . 'resetpass.php?un=' . base64_encode($_POST['username']) . '&token=' . $result->token;
        $resetUrl = '<a title="Password Recovery" href="' . $resetUrl . '">' . $resetUrl . '</a>';
        sendTemplateEmail($_POST['username'], 'recovery', array('url' => $resetUrl));
        $msg= '<p>Login information will be sent if the email address ' . $_POST['username'] . ' is registered.</p>';
    } else {
        $msg = '<p>Sorry, unable to send password reset information. Try again or contact an administrator.</p>';

There’s actually a couple of things wrong here.

The most problematic flaw (and the one that actually caused the security vulnerability) is that LOGIN_APP_URL happens to point to a server that is accessible to the public internet. This means that anyone who wanted to could bypass this script and make a request directly to its resettoken endpoint, supplying any username they want, and get a valid password reset token for that username! Then all they had to do was manually construct the URL for the reset password page with the username and their acquired token to be able to acquire access to that account.

There’s another more subtle issue here, though: why is an HTTP request being made that returns a password reset token in the first place? What should be happening here is that the same code which generates the reset token should also send the email, thus avoiding the need to expose the reset token generator via an external interface in the first place.

(There’s also the slight “wtf” of using PHP’s output buffer to get the results of a cURL call, rather than using CURLOPT_RETURNTRANSFER, but that’s not a security issue.)

Posted on May 8, 2012, in Software Development and tagged , . Bookmark the permalink. Comments Off on Yet Another Way You Shouldn’t Implement Password Recovery.

Comments are closed.

%d bloggers like this: