FormMail Problem

[Olen R. Pearson]

Hi,

I am using FormMail.CGI v. 1.92 (04/21/02) to process s form on a Website.
Though I am not familiar with Perl, I have used this script and its earlier
versions for several years without problems.

Now, the script will process the form for some e-mail addresses but not
others, giving the error message:

Error: Bad/No Recipient
There was no recipient or an invalid recipient specified in the data sent to
FormMail. Please make sure you have filled in the recipient form field with
an e-mail address that has been configured in @recipients.

Here are the pertinent lines from the script:

@referers = ('nursesaregreat.com','fast.net','citcom.net','verizon.net');
@recipients = &fill_recipients(@referers);

and from the form code

<input type=hidden name="recipient" value="(e-mail address removed)">

It seems to work for email addresses at 'nursesaregreat.com', 'fast.net',
and 'citcom.net' but NOT 'veerizon.net'.

Any ideas??

Thanks.c

 

[Gunnar Hjalmarsson]

I am using FormMail.CGI v. 1.92 (04/21/02) to process s form on a Website.
Though I am not familiar with Perl,

Please note that this group is for discussing Perl programming matters,
not for providing help to use particular scripts.

Now, the script will process the form for some e-mail addresses but not
others, giving the error message:

Error: Bad/No Recipient
...

Any ideas??

Ask the script author for assistance.

If you don't get help that way (you probably don't...), try one of the
form-to-mail solutions at http://nms-cgi.sourceforge.net/scripts.shtml
instead.

 

[Christopher Nehren]

If you don't get help that way (you probably don't...), try one of the
form-to-mail solutions at http://nms-cgi.sourceforge.net/scripts.shtml
instead.

Even if you *do* get help that way, *stop* using FormMail now. It's
horribly, horrendously insecure and thoroughly broken.

Best Regards,
Christopher Nehren

 

[Gunnar Hjalmarsson]

*stop* using FormMail now. It's
horribly, horrendously insecure and thoroughly broken.

Even if I'm not sure, I have a feeling that you are referring to
previous versions. If also v. 1.92 deserves that verdict, it would be
nice if you (or somebody else) could explain *how* it's broken.

 

[Anno Siegel]

Even if I'm not sure, I have a feeling that you are referring to
previous versions. If also v. 1.92 deserves that verdict, it would be
nice if you (or somebody else) could explain *how* it's broken.

I don't know about security but it's still substandard Perl code.
I see no strict, no warnings, use of global package variables all over
the place and a general lack of familiarity with Perl's possiblilities.
Some use of qr() would help the code as would POSIX::strftime instead
of the homebrew date conversion.

One concrete example (there are more):

$Config{'required'} =~ s/(\s+|\n)?,(\s+|\n)?/,/g;
$Config{'required'} =~ s/(\s+)?\n+(\s+)?//g;
$Config{'env_report'} =~ s/(\s+|\n)?,(\s+|\n)?/,/g;
$Config{'env_report'} =~ s/(\s+)?\n+(\s+)?//g;
$Config{'print_config'} =~ s/(\s+|\n)?,(\s+|\n)?/,/g;
$Config{'print_config'} =~ s/(\s+)?\n+(\s+)?//g;

That should be something like (untested):

for ( @config{ qw( required env_report print_config} ) {
s/(\s+|\n)?,(\s+|\n)?/,/g;
s/(\s+)?\n+(\s+)?//g;
}

As mentioned I didn't look for holes (other people are presumably
doing that), but the coding standard doesn't instil much confidence.
The vulnerabilities that have been exposed in the past may have been
fixed, but a secure program must be written with security in mind
from the start. Clearly this code hasn't seen a ground-up rewrite
since its beginnings under Perl 4. I wouldn't trust it.

Anno

 

[Gunnar Hjalmarsson]

I don't know about security but it's still substandard Perl code.

I do of course not suggest that it's good code in the light of today's
standards. Not even the author does, btw.

I see no strict, no warnings, use of global package variables all over
the place and a general lack of familiarity with Perl's possiblilities.
Some use of qr() would help the code as would POSIX::strftime instead
of the homebrew date conversion.

One concrete example (there are more):

$Config{'required'} =~ s/(\s+|\n)?,(\s+|\n)?/,/g;
$Config{'required'} =~ s/(\s+)?\n+(\s+)?//g;
$Config{'env_report'} =~ s/(\s+|\n)?,(\s+|\n)?/,/g;
$Config{'env_report'} =~ s/(\s+)?\n+(\s+)?//g;
$Config{'print_config'} =~ s/(\s+|\n)?,(\s+|\n)?/,/g;
$Config{'print_config'} =~ s/(\s+)?\n+(\s+)?//g;

That should be something like (untested):

for ( @config{ qw( required env_report print_config} ) {
s/(\s+|\n)?,(\s+|\n)?/,/g;
s/(\s+)?\n+(\s+)?//g;
}

Okay. But broken? Not just broken, but *thoroughly broken*?

As mentioned I didn't look for holes (other people are presumably
doing that),

I'm waiting.

Clearly this code hasn't seen a ground-up rewrite
since its beginnings under Perl 4.

There were two points I tried to raise by replying to Christopher's post:

1. Perl 4 style programs are not automatically *broken*.

2. When somebody claims that one of the most spread Perl scripts out
there is "horribly, horrendously insecure and thoroughly broken", it's
reasonble to demand that he provides evidence supporting the statement.
If he doesn't, maybe even can't, the claim tells me more about the
judgement of the one who makes the claim than about the code in question.

 

[Anno Siegel]

I don't know about security but it's still substandard Perl code.

[...]

There were two points I tried to raise by replying to Christopher's post:

1. Perl 4 style programs are not automatically *broken*.

Not my point. I mention the Perl 4 style to show that the program
has not been re-written since its Perl 4 days, when it *was* broken.

2. When somebody claims that one of the most spread Perl scripts out
there is "horribly, horrendously insecure and thoroughly broken", it's
reasonble to demand that he provides evidence supporting the statement.
If he doesn't, maybe even can't, the claim tells me more about the
judgement of the one who makes the claim than about the code in question.

Probably the wording doesn't apply to the current code anymore. (I wasn't
aware of the update myself.) However it must still be considered insecure
because it wasn't designed for security (provably), and hasn't been
redesigned. So these days it's bad Perl, derived from horribly insecure
Perl. I'd still not recommend it

Anno

 

[Damian James]

Not my point. I mention the Perl 4 style to show that the program
has not been re-written since its Perl 4 days, when it *was* broken.


Probably the wording doesn't apply to the current code anymore. (I wasn't
aware of the update myself.) However it must still be considered insecure
because it wasn't designed for security (provably), and hasn't been
redesigned. So these days it's bad Perl, derived from horribly insecure
Perl. I'd still not recommend it

I am baffled slightly that there's been an ongoing discussion of one
of MW's old broken cgi scripts here without mention being made of the
NMS project. I did have the understanding that there are versions of
"FormMail.cgi" there that one ought to be able to recommend to neophytes
with moderate confidence...

Google took me to: http://nms-cgi.sourceforge.net/scripts.shtml

Hope that's of some help to the OP. I'd strongly implore that he not use
any extant original Formmail script without at least looking at the much
better versions available.

--damian

 

[Gunnar Hjalmarsson]

I am baffled slightly that there's been an ongoing discussion of one
of MW's old broken cgi scripts here

The discussion has been about whether it's accurate to describe version
1.92 of Matt's FormMail script as "horribly, horrendously insecure and
thoroughly broken" without providing any evidence. I notice that also
you refer to it as "broken", but that does not make a more accurate
description just because it's being repeated, does it?

without mention being made of the NMS project.

NMS was mentioned.

I did have the understanding that there are versions of
"FormMail.cgi" there that one ought to be able to recommend to neophytes
with moderate confidence...

Google took me to: http://nms-cgi.sourceforge.net/scripts.shtml

You could have viewed my first reply in this thread instead:
http://groups-beta.google.com/group/comp.lang.perl.misc/msg/c8b264fe6eb8bec8

 

[Eric Schwartz]

The discussion has been about whether it's accurate to describe
version 1.92 of Matt's FormMail script as "horribly, horrendously
insecure and thoroughly broken" without providing any evidence.

Here's some evidence: without getting into whether or not it's a good
idea to write your own CGI-parsing code, the CGI-parsing code in there
doesn't catch a few of the most common error cases (query string
delimited by ; not & for GET, and read not reading all data in the
buffer at once for POST). That qualifies, to me, as broken, at
least. Also, the regex for valid emails will flag as failed a number
of valid addresses.

As for security holes, there's the obvious one that HTTP_REFERER is
easily spoofable. If you're paying attention, you can restrict
somewhat the domains to which a DDOS can be directed, but it's next to
impossible to use without opening yourself up to one.

There's also a lot of ugly syntax and bad coding practises in there
I'd hate to see anybody emulate-- every function is called with
leading &, the whole get_date routine would be better off as
appropriate calls to POSIX::strftime().

So yeah, on further study I'd agree that 'broken' and 'insecure' are
empirically valid adjectives. You may disagree with the adverbs, if
you like; those are clearly value judgements.

-=Eric

 

[Christopher Nehren]

Even if I'm not sure, I have a feeling that you are referring to
previous versions. If also v. 1.92 deserves that verdict, it would be
nice if you (or somebody else) could explain *how* it's broken.

First, no taint checking. Sure, this in itself doesn't make the code
insecure, but it's certainly not helping things.

The lack of 'use CGI;', while similarly not instantly insecure, bothers
me in a similar fashion. Okay, it was written in the days of Perl 4, but
that's no reason that it couldn't have been updated.

I wouldn't rely on something like /usr/lib/sendmail (and realistically
couldn't, if taint checking were enabled), and would opt to go with
a module. This would give one portability in most cases as well as
security (assuming no external program is used).

In &parse_form, the /e operator to s/// is applied on completely
unprocessed CGI variables -- not once, but twice. I don't even want to
think about what that can cause.

Best Regards,
Christopher Nehren

 

[Sherm Pendley]

The lack of 'use CGI;', while similarly not instantly insecure, bothers
me in a similar fashion. Okay, it was written in the days of Perl 4

A weak excuse at best - Perl 4 may not have had CGI.pm, but it *did*
have cgi-lib.pl.

sherm--

 

[Damian James]

The discussion has been about whether it's accurate to describe version
1.92 of Matt's FormMail script as "horribly, horrendously insecure and
thoroughly broken" without providing any evidence. I notice that also
you refer to it as "broken", but that does not make a more accurate
description just because it's being repeated, does it?

I am amazes that you consider it worth anyone's time to do that. It has been
done, and done many, many times. Perhaps not in this thread but over several
years in this newgroup, quite possibly hundreds of times. I refuse to be
specific or enumerate that, and insist that it is worth noone's time to do so
again, and again, and again. Search the archives, for goodness' sake.

...
You could have viewed my first reply in this thread instead:
http://groups-beta.google.com/group/comp.lang.perl.misc/msg/c8b264fe6eb8bec8

Fine. Glad to see that someone had the sense to point that out. Given that
an example of a known-good version of the program exists, why is this being
discussed at all? Move on. Next!



--damian

 

[Gunnar Hjalmarsson]

the CGI-parsing code in there
query string delimited by ; not & for GET,

It's a *form*-to-mail script, so ';' as delimiter is not applicable.

=> Not broken.

and read not reading all data in the buffer at once for POST

Well, it normally does its work as intended...

Also, the regex for valid emails will flag as failed a number
of valid addresses.
True.

As for security holes, there's the obvious one that HTTP_REFERER is
easily spoofable.

<quote from the FormMail docs>
This is not a security check. Referer headers can EASILY be faked.
</quote from the FormMail docs>

=> Not a security hole.

So yeah, on further study I'd agree that 'broken' and 'insecure' are
empirically valid adjectives. You may disagree with the adverbs, if
you like;

I do. It was them that caused me to object.

those are clearly value judgements.

So are the adjectives, to some extent. For a serious and accurate
discussion on programming code, you'd better be careful about which
"value judgements" you use. ;-)

 

[Gunnar Hjalmarsson]

First, no taint checking. Sure, this in itself doesn't make the code
insecure,

Probably not in this particular script. And certainly not "horribly,
horrendously insecure".

The lack of 'use CGI;', while similarly not instantly insecure,
True.

I wouldn't rely on something like /usr/lib/sendmail (and realistically
couldn't, if taint checking were enabled),

Are you saying that you can't open a pipe to a program when tainted mode
is enabled? That is simply not correct.

In &parse_form, the /e operator to s/// is applied on completely
unprocessed CGI variables -- not once, but twice.

Are you talking about

$value =~ s/%([a-fA-F0-9][a-fA-F0-9])/pack("C", hex($1))/eg;

etc.? Note that the /e modifier is applied on the right side of the s///
operator, not the $value variable.

I don't even want to think about what that can cause.

I do recommend that you think about it.


So, is that all there is? I'm stunned.

You'd better be more careful of how you express yourself, man. ;-)

 

[Eric Schwartz]

It's a *form*-to-mail script, so ';' as delimiter is not applicable.

UAs can split up the args however they like. Still broken.

Well, it normally does its work as intended...

Normally most things do, so why should we bother testing anything for
failure? That's poor reasoning if ever I saw it.

<quote from the FormMail docs>
This is not a security check. Referer headers can EASILY be faked.
</quote from the FormMail docs>

=> Not a security hole.

Just because the docs alert you to it doesn't mean it's not a hole.
All it means is that the author is aware of the hole. Also, someone
else pointed out the /e on the regexes, which I didn't notice.

-=Eric

 

[Gunnar Hjalmarsson]

UAs can split up the args however they like. Still broken.

No browser will use anything but '&' in the foreseeable future.

Normally most things do, so why should we bother testing anything for
failure? That's poor reasoning if ever I saw it.

Okay, I admit that.

Just because the docs alert you to it doesn't mean it's not a hole.

How can a feature that is not intended to provide for security be a
security hole??

Also, someone else pointed out the /e on the regexes, which I
didn't notice.

What about it? Did you read my reply?
http://groups-beta.google.com/group/comp.lang.perl.misc/msg/c81511a4fc69d112

 

[Anno Siegel]

[...]

Also, someone
else pointed out the /e on the regexes, which I didn't notice.

Those are harmless (much as I'd like to find fault with FormMail

A substitution "s/xxx/$user_string/e" is not like "eval $user_string"
but similar to "eval qq( $user_string)". That doesn't interpret
$user_string.

s///ee is another matter.

Anno

 

[Gunnar Hjalmarsson]

much as I'd like to find fault with FormMail

Not only you.

And that explains this unnecessary thread just because I pointed out
that "horribly, horrendously insecure and thoroughly broken" is not an
accurate description of the script the OP asked for help with.

 

[Anno Siegel]

Not only you.

And that explains this unnecessary thread just because I pointed out
that "horribly, horrendously insecure and thoroughly broken" is not an
accurate description of the script the OP asked for help with.

You seem to take this as inexplicable random hostility, but there's
a history to it. Matt (personally quite the nice guy, it seems) has,
over many years, ignored pleas from the Perl community to take down,
fix, or get help fixing his archives. That may be understandable from
a young programmer whose crappy programs have hit the market successfully,
(and they were unmitigated crap in the beginning) but the net effect
was a drain of effort from the rest of the community. That isn't taken
lightly, and it is no wonder the efforts at fixing the scripts is seen
as "too little too late".

Anno