Re: smash and grab
- From: "Paul Lalli" <mritty@xxxxxxxxx>
- Date: 3 Aug 2006 10:06:45 -0700
Tim Wolak wrote:
I'm working on a bit of code to parse a logfile, grab the IP's and put
them in the deny file. In the beginning of my code I'm grabbing all the
IP's in the deny file, putting them in an array to check against later
to make sure that IP is not already in the file. I can get all the
information I need but when I do a comparison of the IP's grabbed from
two files when there is a match is won't print the match!
#!/usr/bin/perl -w
#use strict;
Why are you commenting this out? If strict is causing your program to
fail, *fix* your program. This is the equivalent of plugging your
fingers in your ears, shouting "LA LA LA I CAN'T HEAR YOU!!"
use IO::Handle;
Why? You never use this module.
my $logfile = "/var/log/messages";
my $secv = "/var/log/secv";
my $hosts = "/etc/hosts.deny";
my $cody = "/etc/hosts.deny";
my @boxes;
my $box;
open(LOG, "$logfile") || die "Cannot open logfile for reading: $!";
1) Do not needlessly double quote variables. See: perldoc -q quoting
2) Use the three argument form of open
3) Use lexical filehandles, not globalbarewords.
open my $LOG, '<', $logfile or die "Cannot open logfile for reading:
$!";
open(SEC, ">$secv") || die "Can't open file!: $!";
open(HOST, "$hosts") || die "Can't open file!: $!";
Same goes for these.
#open(DEAD, ">$cody") || die "Can't open file!: $!";
Same would go for this, if it wasn't commented out.
foreach (<HOST>) {
This construct "slurps" the entire file into memory, only to then
process the file's contents line by line. There is no need for that.
Change the foreach to a while.
if($_ =~ /(\d+\.\d+\.\d+\.\d+)/) {
Don't put parentheses in your regular expression if you're not using
the captured submatch.
push (@boxes, $_);
}
Please fix your indentation.
else {
next;
}
This is completely pointless, as there is no other code to "next" over.
}
This entire block could have been written:
my @boxes = grep /\d+\.\d+\.\d+\.\d+/, <HOST>;
but *should* have been written:
my @boxes;
while (<HOST>) {
push @boxes, $_ if /\d+\.\d+\.\d+\.\d+/;
}
close HOST;
while (<LOG>){
if($_ =~/Failed password for invalid/) {
print SEC "Invalied user logon attempt!:$_\n";
if(/(\d+\.\d+\.\d+\.\d+)/) {
I don't understand your full goal, so may be this is what you
intended, but you do realize you're only checking for the IP address if
the current line has this "Failed" substring, right? Is that really
what you wanted?
$tim = $1;
foreach $box (@boxes) {
if($box =~ m/"$tim"/){
Here, I'm guessing is your actual problem. You are checking to see if
$box contains a double-quote character, followed by $time, followed by
a double quote character. Did you mean for there to be double quotes
in that pattern match? Probably not.
Also, why are you checking to see if $box *contains* the IP address.
Didn't you fill @boxes with strings that are just IP addresses? You
should be checking for equality, not matching.
print "Match:$tim\n"
} else {
print "No Match:$box\n";
}
perldoc -q contain
will show you how you *should* be checking to see if a list of elements
contains a certain element. Basically, you should be using a hash, not
an array.
Change the first while loop's body from
push @boxes, $_ if /\d+\.\d+\.\d+\.\d+/;
to:
$boxes{$_} = 1 if /\d+\.\d+\.\d+\.\d+/;
And replace this entire inner for loop with:
if (exists $boxes{$tim}) {
print "Match: $tim\n";
} else {
print "No Match: $tim\n";
}
}
}
}
}
Paul Lalli
.
- References:
- smash and grab
- From: Tim Wolak
- smash and grab
- Prev by Date: Re: smash and grab
- Next by Date: Re: smash and grab
- Previous by thread: Re: smash and grab
- Next by thread: Re: smash and grab
- Index(es):
Relevant Pages
|
