|
| Category: |
Utility Scripts |
| Author/Contact Info |
Pete Jewell <pete@maverick-dbms.org> |
| Description: |
This is my second perl program, written to fulfill a need at work. I was wondering if someone with more experience could critique it, there being no other perl programmers in my office :-)
It's aim is to be run as part of the .bash_profile, to remove any prior D3 program running from the user's IP, to solve the problem of rebooted clients at a customer of ours (stopping them rebooting wasn't an option).
Oh, almost forgot, it's also supposed to run suid root, as the users often use different login names from the same workstation. |
#!/usr/bin/perl
use strict;
use Sys::Syslog;
use English;
my ($temp); # holds temporary data
my ($mytty); # holds my tty
my ($ttytype); # holds either tty or pts
my ($ipaddr); # holds my ip addr
my ($childpid); # holds the pid of a child process
my (@w_listing); # array for output of w command
delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};
$ENV{PATH} = "/bin:/usr/bin";
open (FROMKID, "-|") or exec ("who", "-m") or system_log ("Can't run w
+ho: $!");
if (eof FROMKID) {exit 1;}
while (<FROMKID>) {
if (/tty(.*?)\s/) {
$mytty = "tty" . $1;
$ttytype = "tty";
}
elsif (/pts(.*?)\s/) {
$mytty = "pts" . $1;
$ttytype = "pts";
}
}
close FROMKID;
if (defined($mytty)) {
open (FROMKID, "-|") or exec ("w", "-s") or system_log ("Can't run w
+: $!");
if (eof FROMKID) {exit 1;}
while (<FROMKID>) {
my ($w_idx); # subsript for @w_listing;
$w_idx = @w_listing;
$w_listing[$w_idx] = $_;
}
close FROMKID;
foreach (@w_listing) {
if (/$mytty\s.*?\b(.*?)\s/) {
$ipaddr = $1;
}
}
if (defined($ipaddr)) {
foreach (@w_listing) {
my ($checktty); # tty to check for d3 process
if (/$ttytype(.*?)\s.*?$ipaddr/) {
next if (($ttytype . $1) eq $mytty); # we don't want to kill o
+urselves!
$checktty = $1;
open (FROMKID, "-|") or exec ("ps", "awwww") or system_log ("C
+an't run ps: $!");
if (eof FROMKID) {exit 1;}
while (<FROMKID>) {
foreach (<FROMKID>) {
if (/^ *(\d*?)\s.*?\b$checktty\s.*?d3.*?VIDEO/) {
my ($killresult);
$killresult = kill 15, $1;
if ($killresult) {
if (kill 0, $1) {
$killresult = kill 1, $1;
}
}
if ($killresult) {
system_log ('PID %d has been killed successfully for %
+s', $1, $ipaddr);
} else {
system_log ('Attempt to kill PID %d for %s failed', $1
+, $ipaddr);
print "You are unable to login because there is alread
+y a D3 process\n";
print "running from your IP address.\n\n";
print "Please call Example Systems for assistance.\n\n
+";
print "Press <ENTER> to continue...";
my $hit_enter = <STDIN>;
exit 1;
}
}
}
}
close FROMKID;
}
}
} else {
print "I cannot determine my IP address.\n\n";
print "Please call Example Systems.\n\n";
print "Press <ENTER> to continue...";
system_log ("Cannot determine IP address");
my $hit_enter = <STDIN>;
}
} else {
print "I cannot determine my tty.\n\n";
print "Please call Example Systems.\n\n";
print "Press <ENTER> to continue...";
system_log ("Cannot determine tty");
my $hit_enter = <STDIN>;
}
sub system_log {
Sys::Syslog::setlogsock ('unix');
openlog ('onlyone', 'pid','user'); # open a channel to the syslo
+gd
syslog ('info', @_);
closelog();
}
|
|
RE: onlyone
by Fastolfe (Vicar) on Oct 31, 2000 at 02:14 UTC |
- open (FROMKID, "-|") or exec ("who", "-m") or system_log ("Can't run who: $!");
You probably want to die here, otherwise both processes (open here does an implicit fork) would continue to run in the event your exec fails.
- Regular expressions like /tty(.*?)\s/ are probably a little more efficient written like /tty(\S+)/.
- if (eof FROMKID) {exit 1;}
Instead of exiting, consider die (or croak with the Carp module, if appropriate), which would give you the opportunity to describe the reason you're aborting the script:
die "File is empty" if eof FROMKID;
You can set $! if you need control over the exit value beforehand.
- $killresult = kill 15, $1;
Your code will be a bit more readable if you were to use, say, 'TERM' instead of 15.
- ... or system_log ("Can't run ps: $!");
While I don't see any immediate problem, since $! probably won't be touched by the user, you don't generally want to call syslog with only a single (well, this second) argument, because it leaves you open to format-based attacks. If you only have a single message that might contain user-supplied data (or anything, even incidental, that could have something that looks like a sprintf-style format), you want to call syslog like this:
syslog('info', '%s', $message);
Which means you'd modify your call to system_log thus: system_log('%s', "Couldn't exec ps: $!"). Again, this doesn't look like a problem here, but you should get into the habit so you don't accidentally leave yourself open in the event your code is vulnerable. Taint-checking under Perl (the -T flag) would spot this problem beforehand (tainted data), I think.
- my $hit_enter = <STDIN>;
If you're just discarding the input anyway, just call <STDIN> in a void context:
<STDIN>;
- I'm curious what you're using the English module for..? You don't seem to be using the "long" versions of any system variables. Kudos for using strict, but I'd be most impressed if your script ran warning-free with the -wT options (even if you don't need to use them in your end product).
Hope this helps!
| [reply]
[d/l]
[select] |
|
| [reply]
[d/l]
[select] |
|
my $pid = open(FROMKID, "-|");
# There are now two processes running, one with
# $pid set to the PID of the child, and the other
# with $pid set to 0 (or undefined upon an error).
unless ($pid) {
if (!defined($pid)) {
system_logger("Unable to fork: $!");
die "fork: $!"; # or just exit as you were
}
# note that in this test, $pid is undefined,
# which means we're the child thread, so no
# matter what happens, we must exec or exit/die
unless (exec(...)) {
system_logger("Cannot exec ...: $!");
die "exec ...: $!"; # or just exit
}
}
is (the use of 'TERM' in kill) a product of the English module, or built in?
It's built-in.
Hope this helps. | [reply]
[d/l] |
Back to Code Catacombs
|
|
