SitePoint Sponsor

User Tag List

Results 1 to 7 of 7

Hybrid View

  1. #1
    SitePoint Guru augathra's Avatar
    Join Date
    Jul 2004
    Location
    united states
    Posts
    826
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Basic Authentication

    I wrote a login system and i'm hoping someone will check it for flaws.

    PHP Code:
    require_once 'includes/common.inc.php';
    require_once 
    'includes/connect.inc.php';
    session_start();
    $action addslashes($_GET['action']);
    if(
    $action == 'logout') {
            
    session_destroy();
    }
    $status '';
    $signin $_POST['signin'];
    if(
    eregi('[~`!@#$%^&*()-+=|\'";:.,?/\\]'$_POST['login'])) {
        
    $invalid '<font color="#FF000"><b>Invalid characters.</b></font>';
    }
    if(!
    $invalid) {
    $login stripslashes($_POST['login']);
    $password addslashes($_POST['password']);
    $password md5($password);

    if(
    $signin) {
        if(empty(
    $login)) {
            
    $status .= '<font color="#FF0000"><b>Empty login, please fill in the text box.</b></font><br>';
            
    $continue false;
        }
        if(empty(
    $_POST['password'])) {
            
    $status .= '<font color="#FF0000"><b>Empty password, please fill in the text box.</b></font><br>';
            
    $continue false;
        }
        if(
    $continue !== false) {
            
    $SELECT "SELECT * FROM users WHERE login = '$login' AND password = '$password' LIMIT 1";
            
    $QUERY mysql_query($SELECT) or die($error);
            if(
    $a mysql_fetch_array($QUERY)) {
                
    $user_id $a['id'];
                if((
    $a['login'] !== $login) && ($a['password'] !== $password)) {
                    
    $session_create false;
                    
    $status .= '<font color="#FF0000"><b>Invalid login/password combination. Please try again.</b></font><br>'
                } else {
                    
    $_SESSION['user_id'] = $user_id;
                    
    Redirect('http://mysite.com/');
                }
            } else {
                
    $status '<font color="#FF0000"><b>Invalid login/password combination. Please try again.</b></font><br>';
            }
        }
    }

    Last edited by augathra; Nov 20, 2004 at 17:51.

  2. #2
    SitePoint Guru augathra's Avatar
    Join Date
    Jul 2004
    Location
    united states
    Posts
    826
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Also, if anyone else has a basic login sys they wanna show, please do so, and I realize there are many out there.

  3. #3
    011521 dbalsdon's Avatar
    Join Date
    Feb 2003
    Location
    North Of Scotland
    Posts
    444
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    have you tried testing it yet? if so, were there any problems with it, did any errors appear??? could you maybe post them, so that we know what to look for?
    Daniel Balsdon
    My Site

  4. #4
    SitePoint Guru augathra's Avatar
    Join Date
    Jul 2004
    Location
    united states
    Posts
    826
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    No, everthing seemed fine. It's a login system, so I am really looking for hacking flaws.

  5. #5
    SitePoint Enthusiast
    Join Date
    Nov 2004
    Location
    Boston
    Posts
    25
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hi

    I don't know what this script is part of, but I get the sense that your service contains multi pages, so I think some logic needs to be adjusted!

    One of the most important things you should always do, is to make your code easy to modify or update without the need to update every script in the service!

    Session control is the heart of any protected service, by centralizing your sessions control you will make your total script better, because keeping each major control separate will help keep your code truly organized!


    Lets look at your script!

    Start with this line...

    PHP Code:
    session_start(); 
    This should be replaced and placed in a include file! Where your session control will be maintained!

    session control should contain the following....

    1. is there already a active session?
    2. if there is should they redirected to a service page?
    3. is the session cookie based or is it passed via a URL query!
    4. if it's URL based, you must define the session strings that will need to be added to all URL strings to maintain the session!
    5. if there is no session create a new one
    6. then begin the session!

    By doing this you will cover all the logic that your service needs and also allow for updates, like changing over to database controlled sessions!


    Now lets look at other things in your script!

    PHP Code:
    $action addslashes($_GET['action']); 
    if(
    $action == 'logout') { 
            
    session_destroy(); 


    assigning a $var to a already declared $var is wasteful and not needed! Also always assign the session to an empty array() before destroying it!

    PHP Code:
    if ( isset ( $_GET['action'] ) && $_GET['action'] == 'logout' )
    {
        
    $_SESSION = array ();
        
    session_destroy (); 

    Again, assigning a $var to a already declared $var is wasteful and not needed!

    every time you do this, you are using another memory address

    PHP Code:
    $signin $_POST['signin']; 
    Try to use Perl type regex(s), they are more system friendly

    PHP Code:
    if(eregi('[~`!@#$%^&*()-+=|\'";:.,?/\\]'$_POST['login'])) { 
        
    $invalid '<font color="#FF000"><b>Invalid characters.</b></font>'

    also you did not define 'invalid', which allows for anyone to pass it a value to your script! Which also would allow anyone to by-pass part of your script!

    PHP Code:
    $invalid '';

    if ( !
    preg_match "/^[a-z0-9_]+$/is"$_POST['login'] ) )
    {
        
    $invalid '<font color="#FF000"><b>Invalid characters.</b></font>';

    This next if() could be by-passed because 'invalid' was not defined!

    PHP Code:
    if(!$invalid) { 
    There are other problems, including your database query, and more un-needed assign of var(s)!


    J!

  6. #6
    SitePoint Guru augathra's Avatar
    Join Date
    Jul 2004
    Location
    united states
    Posts
    826
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thank you for the reply! I have a few questions i hope you will answer.

    also you did not define 'invalid', which allows for anyone to pass it a value to your script! Which also would allow anyone to by-pass part of your script!
    If the !eregi returned true, $invalid would not be empty anymore. The next piece is if(!$invalid) {}, which was supposed to be if(!empty($invalid)) { }.
    Wouldn't this prevent the user from putting in those characters?

    I am fairly new to $_SESSION stuff, so newbie session question.
    is there already a active session?
    The only thing i'm looking for is $_SESSION['user_id'], wouldn't this check for active sessions?

    If you meant not making them login if one already exists, I still had to add that. I should have stated that.

  7. #7
    SitePoint Enthusiast ssx-gun's Avatar
    Join Date
    Sep 2002
    Location
    Strongsville, OH
    Posts
    97
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by augathra
    If the !eregi returned true, $invalid would not be empty anymore. The next piece is if(!$invalid) {}, which was supposed to be if(!empty($invalid)) { }.
    Wouldn't this prevent the user from putting in those characters?
    Its out of scope right now. Define it before the if statement
    eg
    PHP Code:
    $invalid "";
      if (... 
    yea it was already posted srry didnt see but yea its out of scope it had to be defined before it goes into the if.
    PHP: Pills Help People
    ---
    weird-one.com



Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •