Solved

Securing a Perl login script

Posted on 2004-04-06
14
12,995 Views
Last Modified: 2013-12-25
I'm not an expert in Perl by any means, but here I went ahead and wrote a tiny login script.  I would like to make the script as secure as possible, both from a malicious user entering in bogus data, and from my own dumb self :)

Standard CGI/Perl stuff here.  Later on I'm going to implement some timing checks against brute force attacks, but I'm not too worried about such a thing, honestly.  I'm also not incredibly concerned with efficiency, but if there is some *major* noob mistake I have made, feel free to point it out and I'll make sure you get some points. Point out as little or as much things as you'd like that is wrong with this script, and I will split the points accordingly.

So without further ado, here it is (I apologize for the poor use of style. I'll get better as I go along). Thank you for your time.


#!/usr/bin/perl

use strict;
use Crypt::PasswdMD5;
use CGI 'param','header';

$CGI::POST_MAX = 128;
$CGI::DISABLE_UPLOADS = 1;

open PWFILE, 'passwords'      # Each line is of "user:MD5" style.
  or die "can't open it";
my @pwfile = <PWFILE>;      # Slurpie it! - not a terribly large file, so no worries.
close PWFILE;

my $user = lc(param("user"));
my $pass = param("pass");

print header( {-type=>'text/plain',-expires=>'now'} );

for (@pwfile) {
     if (
          m/
               ^$user:               # Line begins with "$user:".
               (\$1\$.{8}\$)     # Backreference 1: 12 character salt value: "$1$xxxxxxxx$".
               (.{22})               # Backreference 2: 22 character encrypted password.
               $                    # Line ends.
          /ox
          && $1.$2 eq unix_md5_crypt($pass,$1)     # When we match $user, check the password.
     )
     {
          print "Authorized...";     # Do some quick/short stuff here.
          exit;                              # No more need to continue the search.
     }
}
print "Incorrect username or password."; # If you can read this, then we must not have found a match!
0
Comment
Question by:BrianPap22
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 5
  • 3
  • 2
  • +3
14 Comments
 
LVL 4

Author Comment

by:BrianPap22
ID: 10770586
Should I take the lack of responses as a sign that I did something right for once? I find that hard to believe for this, my second Perl script ever.

Would a working prototype help someone? If so, just let me know and I can throw one up on some webspace.

OK, new policy... I won't split the points.  500 points per valid suggestion (I'll just create a new award question). If this is not acceptable under the ToS of EE, then a moderator may kindly let me know, and I will not do that. Otherwise, I have unlimited question points and I'm itchin' to use them :)
0
 
LVL 48

Expert Comment

by:Tintin
ID: 10770872
That's a damm good effort for a Perl newbie (I suspect you must have other programming experience).

There's no glaring holes.  Just a couple of comments:

1.  You should add a check to see if $user and/or $pass are not specified, otherwise there's no point in proceeding.  So before the open, do a:

die "No username supplied\n" unless $user;
die "No password supplied\n" unless $pass;

2. Also your for loop could probably be written to be a little more readable (although there's nothing really wrong with it). As I can figure, the password file has entries along the lines of:

username:xxxxxxxxxxxxzzzzzzzzzzzzzzzzzzzzzz

Where x is the salt and z is the encrypted password?  

for (@pwfile) {
  if (/^$user:/) {
      my ($salt,$encpass) = /:(.{12})(.{22})/;
       if ($salt$encpass eq unix_md5_crypt($pass,$salt) {
           print "Authorized\n";
           exit;
       }
  }
}

print "Incorrect username or password\n";    

0
 
LVL 4

Author Comment

by:BrianPap22
ID: 10771223
An example line in the pass file would look like this (bogus of course :)

brian:$1$CuRbwNsF$ibVNqwFpWc9o5fJxwziTFT

$1$CuRbwNsF$ is the salt. The rest is the encoded password. The salt is actually 8 characters, but it always begins with "$1$" and ends with "$".  When you encrypt the plaintext password together with the salt value, it returns the salt value as part of the return (odd that it does it that way, but that's just the way it is).

When I wrote the for() loop, I knew I was going to ask for advice, and I thought the way I did it would actually be the most readable. Of course, that was just my opinion.  But you suspect correctly; I do have other experience. PHP mostly, but I'm a little new to that, also.  I originally wrote this script in PHP, but all of a sudden my needs changed, and I needed it in Perl.

Funny that I forgot to check for empty values. I was trying to keep it as simple as possible for readability.
0
Is Your DevOps Pipeline Leaking?

Is your CI/CD pipeline a hodge-podge of randomly connected tools? You’ve likely got a tool to fix one problem & then a different tool to fix another, resulting in a cluster of tools with overlapping functionality. Learn how to optimize your pipeline with Gartner's recommendations

 
LVL 48

Expert Comment

by:Tintin
ID: 10771286
In that case, change my regex to

my ($salt,$encpass) = /:\$1\$(.{8})\$(.{22})/;
0
 
LVL 84

Accepted Solution

by:
ozo earned 500 total points
ID: 10771744
if (/^$user:/) { #beware of user=.*|
0
 
LVL 4

Author Comment

by:BrianPap22
ID: 10772423
I completely missed that, ozo. I think I'll take the pro-active way and limit their accounts to just letters, numbers, dash and underscore, and that will be that.
0
 
LVL 51

Expert Comment

by:ahoffmann
ID: 10777752
> $CGI::POST_MAX = 128;
is more or less useless, unfortunately, but its good to be there anyway ;-)
Reason:  when perl gets called, the web-server already collected all data

As ozo already pointed out: you need to clean all incomming variables ($user, $pass)
Don't know what
  unix_md5_crypt($pass,$1)
does, but if it calls system commands somehow, then I'd try to give as password:
   ';/bin/rm -rf *.*
or:
  |/bin/rm -rf *.*

So please check all parameters using white lists, like:
   $user =~ s#[^a-zA-Z0-9.-]##g;
   $pass =~ s#[^a-zA-Z0-9.-]##g;
take care when adding more characters for the password.
0
 
LVL 4

Author Comment

by:BrianPap22
ID: 10783462
It does not appear to me that unix_md5_crypt() calls system commands.

Read about it here:
http://search.cpan.org/~luismunoz/Crypt-PasswdMD5-1.3/PasswdMD5.pm
"Crypt::PasswdMD5 - Provides interoperable MD5-based crypt() functions"
0
 
LVL 51

Expert Comment

by:ahoffmann
ID: 10785024
>  It does not appear to me ..
you know for shure, or not?
Are you shure that underlaying systems (like Digest::MD5) behave this way in future too?
If not, make shure your parameters won't harm.
That's my message, you asked for security .. ;-)
0
 
LVL 4

Author Comment

by:BrianPap22
ID: 10787343
I understand what you're saying.  It probably won't change in the future, but I suppose it won't hurt to prepare for the worst.

I know it's better to use "white lists" instead of "black lists."  So my list of "approved characters" so far is as follows:

a-z
A-Z
0-9
,.<>[]{}?_+
~!@#$%^&*()_+

This should exclude [space], backtick [`], [/\] forward and backslash, pipe [|], colon and semi-colon [;:], and everything else I can't think of.

My user's need to be able to enter in as sophisticated a password as possible, so I don't want to go overkill with this.

You know, I'm starting to get the feeling that I shouldn't be using regular expressions at all. If I set up a for() loop to run through each line and used ($uname,$hash) = split(/:/), then I could just match 'if $uname eq $user'  ... etc.

Might that be safer? Worse?  No difference?  Remember, efficiency is not a priority.
0
 
LVL 84

Expert Comment

by:ozo
ID: 10787451
eq $user is more efficient, but otherwise equivalent to =~ m/^\Q$user\E\z/ which are of course safer and more forgiving than m/$user/
0
 
LVL 51

Expert Comment

by:ahoffmann
ID: 10789798
with %  you pass over URL encoded charaters
with !  you may access shell commands
with &  you can combine shell commands
with $  you can access variables in shell
with @  you may access functions in M$ SQL
with %_ you have wild cards in SQL
and so on ...

My pedantic filter for a username is:
   a-zA-Z0-9.-
for the password, it needs to be more, I agree
0
 
LVL 2

Expert Comment

by:mishagale
ID: 10835799
I'll join Tintin on contratulating you for such a nifty first script, and there is only one nit I want to pick:
After your if (regex) { } you have nothing to handle the exception that the format of the password file is invalid (the regex itself is not matched). This is not  actually insecure, as this case would still result in failed authorization, but it is confusing for a user, and hard for an admin to debug, if their correctly typed password should fail due to a corrupted file.
0
 
LVL 3

Expert Comment

by:mastallama
ID: 13466763
your script is awesome!

MastaLlama
0

Featured Post

Business Impact of IT Communications

What are the business impacts of how well businesses communicate during an IT incident? Targeting, speed, and transparency all matter. Find out more in this infographic.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
Why am i getting "Use of uninitialized value $bo_plus in substitution (s///) 14 107
BATCH to EXE Converter 2 99
convert Systemjs to Webpack 3 126
PowerShell logging 1 33
This article is meant to give a basic understanding of how to use R Sweave as a way to merge LaTeX and R code seamlessly into one presentable document.
The Windows functions GetTickCount and timeGetTime retrieve the number of milliseconds since the system was started. However, the value is stored in a DWORD, which means that it wraps around to zero every 49.7 days. This article shows how to solve t…
Learn the basics of lists in Python. Lists, as their name suggests, are a means for ordering and storing values. : Lists are declared using brackets; for example: t = [1, 2, 3]: Lists may contain a mix of data types; for example: t = ['string', 1, T…
In this fifth video of the Xpdf series, we discuss and demonstrate the PDFdetach utility, which is able to list and, more importantly, extract attachments that are embedded in PDF files. It does this via a command line interface, making it suitable …

738 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question