r_mehmed has asked for the wisdom of the Perl Monks concerning the following question:

Monks,
i'm reeeally confused! I hope someone can help. I know these are all trivial Qs, but i suppose someone must ask them! I have few subs that should handle an admin page, where users can add, delete etc. db entries. my script starts with setting up cookies for user and password. once that is done there is the sub login that checks for valid users and upon errors calls the sub error_page,else it prints a menu using sub la_menu:
sub login {
    my $username    = $q->param('username');
    my $password    = $q->param('password');
    my $SQL    = "SELECT username, password FROM user WHERE username =
+ '$username'";
    my @row;
    @row = $dbh->selectrow_array($SQL);

    unless (exists $row[0]) {
        &error_page;
        exit;
    }

    unless ($password eq "$row[1]") {
        error_page();
    }
    la_menu();
}#end#of#sub#login
sub error_page {
    print    $q->start_html(-title=>"some.com| ERROR", -style=>{src=>"
+../../mycss.css"});
    print<<HTML;
    <p class="edit_header"><strong>some.com | ERROR</strong></p>
    <h2 class="error">Invalid username or password</h2>
    <p class="footer"><a href="http://www.some.com">some.com</a></p>
HTML
} #end#of#sub#error_page

sub la_menu {
    if ($q->param( "add" )) {
        add_new();
    } elsif ($q->param( "edit" )) {
            edit();
    }
    print<<HTML;
    <form method="get" name="la_manu" class="edit">
    <input name="edit" type="submit" value="Edit Properties " class="b
+uttons">
    <input name="add" type="submit" value="Add New" class="buttons">
    </form>
HTML
    
}#end#of#sub#la_menu

[download]
until here it works fine, however once one of the buttons of the menu sub is pressed it always prints the error page. i guess what i am unclear is, if i should check for valid user with every subroutine. i also tried to check if the cookies equal the username and password passed from the login form, the result seems to be the same! this is the sub that gets called when a button in sub la_menu is pressed: 
sub add_new {
    my $user    = $q->cookie('username');
    my $pass    = $q->cookie('password');
    
    if ($cookie eq $user, $cookie1 eq $pass) {
    print<<HTML;
        ...
    HTML
        }else {
        error_page();
    }
}

[download]
hope this is clear?! thanks
r_mehmed

Edited 2003-03-05 by mirod: changed the title

Replies are listed 'Best First'.
Re: cookies
by dws (Chancellor) on Mar 05, 2003 at 07:10 UTC
    OM_Zen points out the direct problem, but you have at least one more. By not using bind parameters when you prepare the query, you're opening yourself to the possibility of someone corrupting your query. A username of 
        gotcha'; delete * from user

    [download]
    could really spoil your day. At the very least, you need to taint-check any input you're getting.

[reply]
[d/l]
      to expand a bit on what dws is trying to say, and to make your code safer, substiute this part of your code: 
      my $SQL    = "SELECT username, password FROM user WHERE username =    
+ '$username'";
my @row;
@row = $dbh->selectrow_array($SQL);

      [download]
      with the following: 
      my $sth = $dbh->prepare("SELECT username, password FROM user WHERE    
+  username = ?");

$sth->execute($username);
# execute substitutes $username in place of the question
# mark above, correctly formatted, with all bad news
# characters removed by the DBI

my @row = $sth->fetchrow;

      [download]
      Hope that helps.
[reply]
[d/l]
[select]
Re: cookies
by OM_Zen (Scribe) on Mar 05, 2003 at 05:38 UTC
    Hi ,

    The form element , it is supposed to be a get or post( I guess you should post) Also have the condition enforded with a && than the , to enforce the logical condition 
    



     if($cookie eq $user && $cookie1 eq $pass){

         print<<HTML

         HTML

     }else{

          error_page();

     }

    [download]


    This can be tried instead of  if ($cookie1 eq $user , $cookie2 eq $pass) ,just on the wake I wanted to say these suggestions
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom