1 (edited by svo 2007-07-28 12:38)

Topic: Improving login security

(Modification for version 1.2.14)

A security hole in PunBB exist when we enter user/password in login.php page. Both are travelling in a open way.:

    POST /forums/login.php?action=in HTTP/1.1
    Host: punbb.org
    .....
    req_username=myname
    req_password=mypass
    login=Login


Database hashes becomes useless when  there is not difficulty in capturing passwords in Intranet, etc. So here there is a way to apply a secure one-way password approach by using Javascript SHA1 and MD5 libraries.
In this way, password is ciphered in the client side:

    POST /forums/login.php?action=in HTTP/1.1
    Host: myhost.com
        .....
    req_username=myname
    req_password=1538cbbd73ce4079230a93bb738b1867
    login=Login


When hash is never the same, it is a much better approach. Note DB hashes are useless when passwords are travelling in plain way.
Administrators must be aware of this.


HOW TO IMPLEMENT:
------------------
Download sha1.js and md5.js.
http://pajhome.org.uk/crypt/md5/sha1src.html
http://pajhome.org.uk/crypt/md5/md5src.html

Put both in the main PunBB folder, where is login.php page.

Make security backups of:
- header.php
- login.php



PAGE CHANGES:
------------

HEADER.PHP:

Find line:
<?php $max=10; ?>

...until line:
function process_form(the_form)

and between them, put this:

<?
/**** START ************************************************/    

// Hashing SHA1 or MD5
$urlogin = $_SERVER['REQUEST_URI']; 

if (preg_match('/login.php/i', $urlogin)) {    

    $sha1_available = (function_exists('sha1') || function_exists('mhash')) ? true : false;        

    if ($sha1_available) {
        $_SESSION["t_parcial"]=sha1(date("l dS of F Y h:i:s A"));        
        echo '<script src="sha1.js"></script>';
        $sha1ok = 1; 

    } else {
        $_SESSION["t_parcial"]=md5(date("l dS of F Y h:i:s A"));                
        echo '<script src="md5.js"></script>';
        $sha1ok = 0; 
    }    
}    

?>

<script type="text/javascript">
<!--
var max=<?php echo $max; ?>;
var randomValue1=Math.floor(Math.random()*max);
var randomValue2=Math.floor(Math.random()*max);
var theSum=randomValue1+randomValue2;
max*=2;

function upAndDown(the_form, sign){
    var theValue=the_form.elements["mathValue"].value;
    if(sign=="+" && theValue<max) theValue++;
    else if(sign=="-" && theValue>0) theValue--;
    the_form.elements["mathValue"].value=theValue;
}



<?     

if ($sha1ok == 1) {     ?>

function encrypt() {
    var sha1_pre=hex_sha1(document.login.req_password.value);
    var sha1=hex_sha1(sha1_pre+"<? echo sha1($_SESSION["t_parcial"] .getenv("REMOTE_ADDR")); ?>")
    document.login.req_password.value=sha1
}

<?        
$eslogin = '';
} else { ?>

function encrypt() {
    var md5_pre=md5(document.login.req_password.value)
    var md5=hex_md5(md5_pre+"<? echo md5($_SESSION["t_parcial"] .getenv("REMOTE_ADDR")); ?>")
    document.login.req_password.value=md5
}

<? 
    $eslogin = '';
} 
?>

/**** END ************************************************/

LOGIN.PHP
Replace all code with this:


<?php
/***********************************************************************

  Copyright (C) 2002-2005  Rickard Andersson (rickard@punbb.org)

  This file is part of PunBB.

  PunBB is free software; you can redistribute it and/or modify it
  under the terms of the GNU General Public License as published
  by the Free Software Foundation; either version 2 of the License,
  or (at your option) any later version.

  PunBB is distributed in the hope that it will be useful, but
  WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  GNU General Public License for more details.

  You should have received a copy of the GNU General Public License
  along with this program; if not, write to the Free Software
  Foundation, Inc., 59 Temple Place, Suite 330, Boston,
  MA  02111-1307  USA

************************************************************************/

if (isset($_GET['action'])) {
    define('PUN_QUIET_VISIT', 1);
}

define('PUN_ROOT', './');
require PUN_ROOT.'include/common.php';


// Load the login.php language file
require PUN_ROOT.'lang/'.$pun_user['language'].'/login.php';

$action = isset($_GET['action']) ? $_GET['action'] : null;

if (isset($_POST['form_sent']) && $action == 'in') {

    session_start();
    
            
    $form_username = trim($_POST['req_username']);
    $form_password = trim($_POST['req_password']);


    $username_sql = ($db_type == 'mysql' || $db_type == 'mysqli') ? 'username=\''.$db->escape($form_username).'\'' : 'LOWER(username)=LOWER(\''.$db->escape($form_username).'\')';

    $result = $db->query('SELECT id, group_id, password, save_pass FROM '.$db->prefix.'users WHERE '.$username_sql) or error('Unable to fetch user info', __FILE__, __LINE__, $db->error());
    list($user_id, $group_id, $db_password_hash, $save_pass) = $db->fetch_row($result);

    $authorized = false;

    if (!empty($db_password_hash))    {

        $sha1_in_db = (strlen($db_password_hash) == 40) ? true : false;
        $sha1_available = (function_exists('sha1') || function_exists('mhash')) ? true : false;

        if ($sha1_available)
            $t=sha1($_SESSION["t_parcial"] .getenv("REMOTE_ADDR"));
        else
          $t=md5($_SESSION["t_parcial"].getenv("REMOTE_ADDR"));
    

        if ($sha1_in_db && $sha1_available && sha1($db_password_hash.$t) == $form_password) 
            $authorized = true;
         else if (!$sha1_in_db && md5($db_password_hash.$t) == $form_password)     
         {
            $authorized = true;

         }
        
     }

    if (!$authorized)  {
             $_SESSION = array();
        session_destroy();
        message($lang_login['Wrong user/pass'].' <a href="login.php?action=forget">'.$xx.$lang_login['Forgotten pass'].'</a>');
       }
    

    // Update the status if this is the first time the user logged in
    if ($group_id == PUN_UNVERIFIED) 
        $db->query('UPDATE '.$db->prefix.'users SET group_id='.$pun_config['o_default_user_group'].' WHERE id='.$user_id) or error('Unable to update user status', __FILE__, __LINE__, $db->error());
    

    // Remove this users guest entry from the online list
    $db->query('DELETE FROM '.$db->prefix.'online WHERE ident=\''.$db->escape(get_remote_address()).'\'') or error('Unable to delete from online list', __FILE__, __LINE__, $db->error());
        
    $expire = ($save_pass == '1') ? time() + 31536000 : 0;
    
    //pun_setcookie($user_id, $form_password_hash, $expire);  
    pun_setcookie($user_id, $db_password_hash, $expire);

       $_SESSION = array();
      session_destroy();
 
      redirect(htmlspecialchars($_POST['redirect_url']), $lang_login['Login redirect']);
    
}


else if ($action == 'out')
{
    if ($pun_user['is_guest'] || !isset($_GET['id']) || $_GET['id'] != $pun_user['id'])
    {
        header('Location: index.php');
        exit;
    }

    // Remove user from "users online" list.
    $db->query('DELETE FROM '.$db->prefix.'online WHERE user_id='.$pun_user['id']) or error('Unable to delete from online list', __FILE__, __LINE__, $db->error());

    // Update last_visit (make sure there's something to update it with)
    if (isset($pun_user['logged']))
        $db->query('UPDATE '.$db->prefix.'users SET last_visit='.$pun_user['logged'].' WHERE id='.$pun_user['id']) or error('Unable to update user visit data', __FILE__, __LINE__, $db->error());

    pun_setcookie(1, random_pass(8), time() + 31536000);

    redirect('index.php', $lang_login['Logout redirect']);
}


else if ($action == 'forget')
{
    if (!$pun_user['is_guest'])
        header('Location: index.php');

    if (isset($_POST['form_sent']))
    {
        require PUN_ROOT.'include/email.php';

        // Validate the email-address
        $email = strtolower(trim($_POST['req_email']));
        if (!is_valid_email($email))
            message($lang_common['Invalid e-mail']);

        $result = $db->query('SELECT id, username FROM '.$db->prefix.'users WHERE email=\''.$db->escape($email).'\'') or error('Unable to fetch user info', __FILE__, __LINE__, $db->error());

        if ($db->num_rows($result))
        {
            // Load the "activate password" template
            $mail_tpl = trim(file_get_contents(PUN_ROOT.'lang/'.$pun_user['language'].'/mail_templates/activate_password.tpl'));

            // The first row contains the subject
            $first_crlf = strpos($mail_tpl, "\n");
            $mail_subject = trim(substr($mail_tpl, 8, $first_crlf-8));
            $mail_message = trim(substr($mail_tpl, $first_crlf));

            // Do the generic replacements first (they apply to all e-mails sent out here)
            $mail_message = str_replace('<base_url>', $pun_config['o_base_url'].'/', $mail_message);
            $mail_message = str_replace('<board_mailer>', $pun_config['o_board_title'].' '.$lang_common['Mailer'], $mail_message);

            // Loop through users we found
            while ($cur_hit = $db->fetch_assoc($result))
            {
                // Generate a new password and a new password activation code
                $new_password = random_pass(8);
                $new_password_key = random_pass(8);

                $db->query('UPDATE '.$db->prefix.'users SET activate_string=\''.pun_hash($new_password).'\', activate_key=\''.$new_password_key.'\' WHERE id='.$cur_hit['id']) or error('Unable to update activation data', __FILE__, __LINE__, $db->error());

                // Do the user specific replacements to the template
                $cur_mail_message = str_replace('<username>', $cur_hit['username'], $mail_message);
                $cur_mail_message = str_replace('<activation_url>', $pun_config['o_base_url'].'/profile.php?id='.$cur_hit['id'].'&action=change_pass&key='.$new_password_key, $cur_mail_message);
                $cur_mail_message = str_replace('<new_password>', $new_password, $cur_mail_message);

                pun_mail($email, $mail_subject, $cur_mail_message);
            }

            message($lang_login['Forget mail'].' <a href="mailto:'.$pun_config['o_admin_email'].'">'.$pun_config['o_admin_email'].'</a>.');
        }
        else
            message($lang_login['No e-mail match'].' '.htmlspecialchars($email).'.');
    }


    $page_title = pun_htmlspecialchars($pun_config['o_board_title']).' / '.$lang_login['Request pass'];
    $required_fields = array('req_email' => $lang_common['E-mail']);
    $focus_element = array('request_pass', 'req_email');
    require PUN_ROOT.'header.php';




?>
<div class="blockform">
    <h2><span><?php echo $lang_login['Request pass'] ?></span></h2>
    <div class="box">
        <form id="request_pass" method="post" action="login.php?action=forget_2" onsubmit="this.request_pass.disabled=true;if(process_form(this)){return true;}else{this.request_pass.disabled=false;return false;}">
            <div class="inform">
                <fieldset>
                    <legend><?php echo $lang_login['Request pass legend'] ?></legend>
                    <div class="infldset">
                        <input type="hidden" name="form_sent" value="1" />
                        <input id="req_email" type="text" name="req_email" size="50" maxlength="50" />
                        <p><?php echo $lang_login['Request pass info'] ?></p>
                    </div>
                </fieldset>
            </div>
            <p><input type="submit" name="request_pass" value="<?php echo $lang_common['Submit'] ?>" /><a href="javascript:history.go(-1)"><?php echo $lang_common['Go back'] ?></a></p>
        </form>
    </div>
</div>
<?php

    require PUN_ROOT.'footer.php';
}


if (!$pun_user['is_guest'])
    header('Location: index.php');

// Try to determine if the data in HTTP_REFERER is valid (if not, we redirect to index.php after login)
$redirect_url = (isset($_SERVER['HTTP_REFERER']) && preg_match('#^'.preg_quote($pun_config['o_base_url']).'/(.*?)\.php#i', $_SERVER['HTTP_REFERER'])) ? htmlspecialchars($_SERVER['HTTP_REFERER']) : 'index.php';

$page_title = pun_htmlspecialchars($pun_config['o_board_title']).' / '.$lang_common['Login'];
$required_fields = array('req_username' => $lang_common['Username'], 'req_password' => $lang_common['Password']);
$focus_element = array('login', 'req_username');


session_start();    

require PUN_ROOT.'header.php';


?>
<div class="blockform">
    <h2><span><?php echo $lang_common['Login'] ?></span></h2>
    <div class="box">
        <form id="login" name="login" method="post" action="login.php?action=in" onsubmit="encrypt()">
            <div class="inform">
                <fieldset>
                    <legend><?php echo $lang_login['Login legend'] ?></legend>
                        <div class="infldset">
                            <input type="hidden" name="form_sent" value="1" />
                            <input type="hidden" name="redirect_url" value="<?php echo $redirect_url ?>" />
                            <label class="conl"><strong><?php echo $lang_common['Username'] ?></strong><br /><input type="text" name="req_username" size="25" maxlength="25" tabindex="1" /><br /></label>
                            <label class="conl"><strong><?php echo $lang_common['Password'] ?></strong><br /><input type="password" name="req_password" size="16" maxlength="40" tabindex="2" /><br /></label>
                            <p class="clearb"><?php echo $lang_login['Login info'] ?></p>
                            <p><a href="register.php" tabindex="4"><?php echo $lang_login['Not registered'] ?></a>  
                            <a href="login.php?action=forget" tabindex="5"><?php echo $lang_login['Forgotten pass'] ?></a></p>
                        </div>
                </fieldset>
            </div>
            <p><input type="submit" name="login" value="<?php echo $lang_common['Login'] ?>" tabindex="3" /></p>
        </form>
    </div>
</div>
<?php

require PUN_ROOT.'footer.php';

code sure be improved although it works.

Re: Improving login security

svo wrote:

So here there is a way to apply a secure one-way password approach by using Javascript SHA1 and MD5 libraries.

You can't use javascript to do anything important, because some UA don't have javascript (enable, or at all).

Anyway, I think security related suggestions and possible hole are supposed to be mailed to Rickard, to avoid giving idea to the script kiddies around.

Re: Improving login security

A SHA1 hash could still be useful...

Re: Improving login security

Already done in 1.3, iirc.

Re: Improving login security

Not this idea specifically though. It is worth thinking about (and it should be easy to make it work for browsers with Javascript and without).

6

Re: Improving login security

To avoid Javascript maybe there is some way to apply a hash before sending password. Perhaps in dependence of the Session unique id? 
I don't know. If somebody want to polish this code or make somehting similar I would be glad to know.

** I have edited the message to add these two lines to keep clean the /tmp directory:

$_SESSION = array();
session_destroy();

regards,

Re: Improving login security

The easiest way would just be to add an onsubmit function that takes the value from password, hashes it, sticks it in hashed_password, and removes password.
That way, if hashed_password is set, we look there for a hash. If not, we look at password for the password. If there's Javascript, we have a hash, and if not there's no hash smile

8

Re: Improving login security

that's a good idea smile

and maybe a function able to work with diferent form variables, because also there is the "change password" option in the Profile section.

Re: Improving login security

Well, the function could be passed the form variable to hash wink

10 (edited by svo 2007-07-28 16:26)

Re: Improving login security

In fact, actually when there is not Javascript activated, the form send passwords normally.
To detect the hashed passwords before the SQL check, I think it could be made by limit the password lenght  (always < of lenght hash)  or by activating the $_SESSION variable in  dependece of Javascript :
<script>
if (true) {
<? $_SESSION["t_parcial"]=sha1(date("l dS of F Y h:i:s A"));?>
}
</script>

the second one sounds more insecure, although allows some message to the visitor. If there is not session, then we can show some warning to the viistor.

Well,  just a pair of ideas. I will try some of both smile

Re: Improving login security

You just send the hashed password in a different variable, it's a fairly common technique that I've seen used in several apps wink