1. You've removed "-w", "-T" and "use strict" from your program. No CGI program should be deployed without using all of those.
2. You've switched from using CGI.pm to a buggy homemade CGI parameter parsing routine.
3. You still send input from the form to an email address that also comes from the form. I told you that you should only ever send fixed content to email addresses that you get from user input.
4. The relevant FAQ suggests including "-oi" in the options that to pass to sendmail for good reason. You ignore that advice.

There's probably more, but these were problems I picked up on in the first minute of looking.

Have you read any of the CGI security documents that merlyn pointed you at?

"The first rule of Perl club is you do not talk about Perl club."
-- Chip Salzenberg

"You still send input from the form to an email address that also comes from the form"... How else can someone tell their friend about my site through my site?!

"You still send input from the form to an email address that also comes from the form"... How else can someone tell their friend about my site through my site?!

By sending an email message that consists of just fixed content. You were doing fine until you decided to include the contents of $message - which could contain anything.

--
<http://www.dave.org.uk>

"The first rule of Perl club is you do not talk about Perl club."
-- Chip Salzenberg

How does this look:

It looks awful.

$foo++  # increment $foo

# Greet the user
print "Hello!\n";

If ($foo =~ /bar/) {  # If $foo contains 'bar'
[download]

Always assume that whoever reads source code either knows the language, or wants to learn it.

Further comments:

• Don't parse form fields yourself if you don't know how to. There are many modules that you can use for this.
• Don't send mail this way. As merlyn already pointed out, ione can easily use your script as an anonymous mailer.
• Don't leave unused subroutines around. It's confusing. If you really want to have the code around, comment it out.
• use strict and warnings.
• Use a module that sends e-mail. My favourite is MIME::Lite (despite its name, it can also send simple plain text messages).
• Learn Perl before writing scripts that are accessible publicly. It's okay to write bad code, but it is not okay to write bad code that others can abuse.

- Yes, I reinvent wheels.
- Spam: Visit eurotraQ.

Use a module that sends e-mail. My favourite is MIME::Lite (despite its name, it can also send simple plain text messages).

You do know that MIME::Lite just uses sendmail don't you? That being the case perhaps you might like to share your rationale for this suggestion.

cheers

tachyon

s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

You do know that MIME::Lite just uses sendmail don't you? That being the case perhaps you might like to share your rationale for this suggestion.

It uses sendmail or Net::SMTP. It fills \n-based security holes by adding whitespace (I believe this is called "folding"), so the mailer doesn't see the new line as a new header.

If you set your To: header to be "foo\@bar.com\nSubject: my own anonymous message\n\nFree pr0n at http://foo.com/!!!\n", MIME::Lite will stop this spammer by doing a simple s/\n/\n /g.

From: real@message.com
To: foo@bar.com
 Subject: my own anonymous message
 
 Free pr0n at http://foo.com/!!!
Subject: real message

This is the real message.
[download]

Sendmail will choke on this 4-line To-header, and the spam is stopped effectively.

Had there not been whitespace in front of the extra lines (note that the line between the fake Subject: and the fake body is NOT empty), the fake Subject: header would have been a REAL header, and the fake body would have been the start of the real body.

This doesn't mean you should don't have to check data yourself - one can still add multiple addresses, and older sendmails send the e-mail regardless of the invalid syntax. But the module does make stupid things a little harder. And it provides nice syntax, calls sendmail in a safe fashion (using an argument list instead of a command string, not that it matters much for constants, but still.). The greatest advantage is that MIME::Lite will let you add an attachment if you later decide to do so. It's a lot harder to do that if you print to sendmail yourself.

- Yes, I reinvent wheels.
- Spam: Visit eurotraQ.

I am not trying to insult anyone. If I were going to give this script to someone who is very new to Perl or CGI (like ME!), this would help them out greatly!!

What I don't understand is, with all the responses I have got on this site, no one can help me fix it. Is it really such a hard task to make this script safe to use?! I am a "Seeker of Perl Wisdom"...

If I were going to give this script to someone who is very new to Perl or CGI (like ME!), this would help them out greatly!!

No, you're just making one of the classic blunders of a beginning programmer (when first told that you should comment your code). Here's an excerpt from an article on anti-patterns in the latest (Feb. 2003) Dr. Dobbs journal:

We are all familiar with the basic duh comment

i++; //add one to i

that is an unsung foundation of antipatternery. But, in fact, it does no more damage than clutter the layout and could even be argued to be useful in that it warns the reader that the module-writer is quite thick.

But don't feel bad...the author goes on to reveal a good commenting blunder of her own...we've all made these mistakes at sometime or another, it's just a matter of (un)learning (from) them.

this would help them out greatly!!

No, it would only stop them from reading documentation. Don't give too much information.

- Yes, I reinvent wheels.
- Spam: Visit eurotraQ.

use CGI::Application::MailPage;
my $mailpage = CGI::Application::MailPage->new(
            PARAMS => {
                         document_root => 'whatever',
                         smtp_server => 'smtp.myserver',
                       },
            );
$mailpage->run();
[download]