2002-11-03 01:02:21
On November 3, 2002 at 01:53, Gunnar Hjalmarsson wrote:

Have you possibly checked out my rewritten script for invoking MHonArc?

Some initial comments about the script (which you are free to
ignore :-):

* The initial BEGIN block is probably not needed.  Fatal messages
  should be sent a log file to avoid any potential sensitive
  information showing up in someone's browser.  Error messages the
  user see should not be some Perl error message, IMHO.

* Since it is known that many people use scripts as-is without
  changing things, I would set the $adminpw variable to a value
  that is not supported.  I.e.  If the script gets a password equal
  to the default value provided in the source distribution, it
  does not allow access.

* It would be nice to have passwords in a separate file, but then it
  would require the user to know how to deny access to those files
  via HTTP.  You may want document that if a user is able to set
  up HTTP authentication via the server, then use that.  Therefore,
  you want to make sure your script supports an empty password,
  or have the ability to run in no password mode.

* I'd sent $pophost to something like "".

* The updatembox() function does not protect against concurrent
  access to $mbox.

* The following statements appear to open up a vulnerability:

  if (!$ENV{'HTTP_USER_AGENT'} or $ENV{'HTTP_USER_AGENT'} =~ /^libwww-perl/) {
      exit (autoupdate());                 # if not invoked from a browser

  What prevents anyone from not causing the autoupdate() routine to
  be invoked?  It appears no password is checked before this statement
  is invoked.

  This appears to allow someone to do a DoS against the the popserver
  ($pophost) or allow arbitrary messages to be posted to the archive.

  FYI, User-Agent strings can be spoofed, so be cautious when testing
  it for a particular agent type.

* The updatembox() could be written to prevent a large amount of
  memory getting used.  At a mimimum, store the passed in message
  data into a single scalar and a pass a reference to it updatembox().
  For example:

    my $newmail = '';
    local $_;
    while (<STDIN>) {
      $newmail .= $_;

  Or better,


  And, updatembox should be written as follows:

    sub updatembox {
      ## XXX: Some kind of file locking should be done here!!
      open (FILE, ">>$mbox") or die "Couldn't open $mbox\n$!"

      my $data = shift;
      if (ref($data) eq 'SCALAR') {
        # Argument is a reference to scalar string
        print FILE $$data;
        print FILE "\n"  unless $$data =~ /\n\Z/;
      } elsif (ref($data) eq 'ARRAY') {
        # Argument is a reference to array
        local $_
        foreach (@$data) {
          print FILE $_;
      } else {
        # Treat argument as a filehandle
        my $line;
        while (defined($line = <$data>)) {
          print FILE $line;
        print FILE "\n"  unless $line =~ /\n\Z/;

  (Routine NOT tested!)

  This version supports giving updatembox() a reference to a scalar
  string, an array, or a filehandle.  This version should support
  the existing pop retrieval code with the slight change of passing a
  reference to the array to updatembox().  Note, an array containing
  each line of a message takes up more memory than a single scalar
  containing the entire message text.

* I do not think the crypt password option provides any benefits.
  If someone sniffs someones cookie, they can just echo it back
  and be authenticated without knowing the real password.

  I guess it does provide some protection if the source of the
  CGI program was accessed and the password cannot be directly
  seen in $adminpw since it is crypted.

* In the shell() routine, you may want to use  It's
  an old library that is still included with perl.  I'm not sure if
  anyone has bothered to make a module out of it.

I'm asking because if you would find the quality to be acceptable, I 
think you should consider linking to or including it in the 
MHonArc distribution.

I do not have a problem with including it in the contrib/ directory
of the distribution.  Of course, keep a version available from
your website so you can update independently from MHonArc releases.
I can list a URL in the contrib/ README to tell people were the
latest version is available.

P.S. You didn't forget about making the maxwidth option also affect 
fixed paragraphs in flowed messages, did you? ;-)

I just checked it in.  It took some time as I discovered a
problem with html_length() being invoked in a substitution portion
of a s/// operation.  A subtle behavior issue dealing with
backreferences.  It would be nice if Perl supported localizing
backreference variables.


To sign-off this list, send email to majordomo(_at_)mhonarc(_dot_)org with the

<Prev in Thread] Current Thread [Next in Thread>