Re: is this script secured enough from internet attacks

In addition to what moritz said:

Perl 4 is dead. An ampersand in front of the function name does something very different in Perl 5 than it did in Perl 4, and usually you do not want that. Get rid of the ampersands.
Your code lacks a lot of error checks.
Why all that line noise in the comments? It makes the code only harder to read.
"close file" before/after a close(HANDLE) is an excellent example for a stupid, useless comment. And there are more examples of this in the code. Comment what is not obvious to the reader, don't repeat the code in english in comments.
What's the purpose of the chmod after closing the file? Neither open nor close should mess with the file's mode.
use CGI::Carp qw/fatalsToBrowser/; exposes information about your script and your environment to an attacker. Remove that from production code.
die "Error 5643: can't print to <br><b>$Results_Path</b><br>$!<hr>"; should not contain HTML.
The hardcoded error number in there is just nonsense, drop it.
The same applies to the other error messages.
Warning_One is a good example for a stupid function name. Does it warn you of a big cat-killing digit 1? Nope! So, name the function according to what it does.
Are you aware that Warning_One exits your script before the emitted HTML is complete? It does not even close or unlock the working file.
In the error message, you expose the name of the file written to an attacker.
The emitted HTML lacks a charset declaration.
The emitted HTML does not end after </html>
The emitted HTML uses obsolete constructs like the attributes in <body>, inline CSS, the B element. Use an external stylesheet and semantic HTML.
You mix code and HTML. Consider using a template instead.
You don't use the three-argument form of open.
You interpolate uselessly, e.g. in chmod (0666, "$Results_Path")
There are more problems with your code, but this list is already too long.

Alexander

--
Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)

Comment on Re: is this script secured enough from internet attacks Select or Download Code

Replies are listed 'Best First'.
Re^2: is this script secured enough from internet attacks by flexvault (Monsignor) on Jun 10, 2011 at 16:21 UTC
Perl 4 is dead. An ampersand in front of the function name does something very different in Perl 5 than it did in Perl 4, and usually you do not want that. Get rid of the ampersands. Could you explain you comment about ampersands? I have never coded in perl 4, but have used perl5 since 1997 on AIX (perl5.00404). I distinguish between the subroutines that I have written and those that are part of a "use Module" from core or CPAN. For example ( without all the code needed to actually work ): `use BerkeleyDB; # CPAN module use Common; # My module my $cursor = $db->db_cursor(); # BerkeleyDB function my $var = &BD_Read($FH,$db,\$key,\$data); # my subroutine BD_Read` [download] I use this to help me distinguish between the source of the code. Is there something wrong with my personal preference? "Well done is better than well said." - Benjamin Franklin	[reply] [d/l]
Re^3: is this script secured enough from internet attacks by Limbic~Region (Chancellor) on Jun 10, 2011 at 18:37 UTC
flexvault, Read perlsub. `NAME(LIST); # & is optional with parentheses. NAME LIST; # Parentheses optional if predeclared/imported. &NAME(LIST); # Circumvent prototypes. &NAME; # Makes current @_ visible to called subroutine.` [download] Unless I am creating a reference to a sub (`my $subref = \&somesub;`), I almost never even think about using & with a sub name purely out of habit. The rare exception is when I am using `goto &somesub;` which @_ gets inherited. Cheers - L~R	[reply] [d/l] [select]
Re^3: is this script secured enough from internet attacks (&) by tye (Sage) on Jun 10, 2011 at 20:58 UTC
My experience is that you will mostly find rather strong reactions against using `&` when calling subroutines. However, the only problem with doing so beside over-zealous style-related reactions is that doing so also disables function prototypes. But, if you use function prototypes, you are also likely to get over-zealous reactions against doing such. So the real problems with using `&` are minimal or none. On the other side, what are the arguments in favor of using `&` ? There is the style argument that you gave. The `&` provides a visual cue that the function being invoked is certainly not a Perl built-in (it can't be). You can also make the style choice of using `&` on functions local to the current package and then not using `&` on functions imported from other modules. It is funny that people who love Perl, including the ugly sigils it pollutes the code with seem to hate the red-headed step-child Perl sigil of `&`, despite it offering the same style / visual benefits. But is there a benefit to using `&` other than style? Actually, there is. `#!/usr/bin/perl -w use strict; sub dump { require Data::Dumper; my $d= Data::Dumper->new( \@_ ) # The defaults are "all wrong": ->Indent(1)->Useqq(1)->Terse(1)->Sortkeys(1); return $d->Dump(); } print &dump( @ARGV );` [download] Since I didn't use a function prototype, surely I can just drop that "ugly" `&` and nothing changes. Note that the above problem doesn't happen for: `use A::Module qw< dump >; print dump( @ARGV );` [download] actually reinforcing the above style choice of only using `&` on subs local to (defined in, not imported to) the current package. I solve that problem by capitalizing all of my subroutine names. Of course, that just leads to me learning that there are quite a few people with rather violent style-based reactions against the dreaded and ugly CamelCase. But at least I get to pick which group grumbles at my code. ;) - tye	[reply] [d/l] [select]
Re^4: is this script secured enough from internet attacks (&) by Jenda (Abbot) on Jun 11, 2011 at 01:13 UTC
The problem with the ampersand is that most people expect that just like `&foo(1,2,3)` behaves the same as `foo(1,2,3)` `&foo;` behaves the same as `foo;` and it does not. It's too easy to remove all your arguments and having the subroutine fiddle with your @_ instead of getting no arguments. Jenda Enoch was right! Enjoy the last years of Rome.	[reply] [d/l] [select]
Re^5: is this script secured enough from internet attacks (&) by tye (Sage) on Jun 11, 2011 at 01:32 UTC
Re^3: is this script secured enough from internet attacks by Anonymous Monk on Jun 10, 2011 at 17:41 UTC
Is there something wrong with my personal preference? Its like wearing a tutu when you're not a ballerina ? :)	[reply]
Re^3: is this script secured enough from internet attacks by ikegami (Patriarch) on Jun 10, 2011 at 18:28 UTC
Is there something wrong with my personal preference? Yes. You said you would add a "`&`" to `$db->db_cursor()` if it was your own module, but it's impossible to do so.	[reply] [d/l] [select]
Re^2: is this script secured enough from internet attacks by Anonymous Monk on Jun 12, 2011 at 18:28 UTC
The emitted HTML uses obsolete constructs like the attributes in <body>, inline CSS, the B element. Use an external stylesheet and semantic HTML. While I agree that you've stated the current trend, the existing standards are HTML 4.01 and CSS 2.1 which permit all of the above. Heck, the only thing that's even deprecated is the use of attributes in the body tag. You're just flat wrong about inline CSS and B. The proposed HTML 5 standard includes both the style attribute and B element.	[reply]
Re^2: is this script secured enough from internet attacks by tercoz (Acolyte) on Nov 01, 2011 at 04:28 UTC
I started to work with another website and I shall look at those remarks you gave me and apply your tips to the new project, thank you	[reply]