2002-05-29-15:55:09 Brian D. Hamm: > No harsh programming statements please this was my first crack at Perl > :)
I'll avoid any harsh statements:-).
Your code is not particularly idiomatic perl, but it's exceptionally
clear, and as far as I can see correct. You note in a comment one
spot where you have a weakness:
> # ARG[5] Complain if this arg does not begin with /var/log/
> # SECURITY ISSUE: need to lock down further, /var/log/../../otherdir
> would succeed
> $log_substr = substr ("$rsync_argv[5]", 0, 16);
> unless ($log_substr eq '/var/log/rotate/') {
> print SSHOUT ("ARG[5] Failure\n");
> $ok = $FALSE;
> }
If you wrote:
if ($rsync_argv[5] =~ m#/\.\./# or $rsync_argv[5] !~ m#^/var/log/rotate/#) {
print SSHOUT ("ARG[5] Failure\n");
$ok = $FALSE;
}
that would address the security problem; and doing a pattern match
is more idiomatic, and perhaps clearer, than doing a string compare
against a counted substr.
Later you have problems getting output flushed to your SSHOUT handle
before you run a command; you can fix that by going
use IO::File;
and somewhere before where you want that data flushed --- perhaps
right after you open the file, since performance isn't an issue here
--- going
SSHOUT->autoflush(1);
to ensure that the handle is flushed after each write.
You'll still need to use system() instead of exec() if you want to
be able to log the success or failure of the command afterwards.
-Bennett
msg04246/pgp00000.pgp
Description: PGP signature
