I was wondering if there are any security concerns in my .pl file?

Yes - as LanX already alluded to, this is susceptible to a Cross Site Scripting (XSS) attack. One way to help mitigate this is to escape any strings you're inserting into the HTML, for example with HTML::Entities. Other comments:

• Always Use strict and warnings!
• The whole @pairs code seems unneccessary, it looks like you've copied some really old Perl form parsing code from somewhere, but this is not needed because you're already using CGI.pm which does this for you.
• You're not checking open for errors - "open" Best Practices
• You're not locking your drivers.html file while you are editing it, which could cause problems when multiple requests are submitted at the same time (though I don't know how flock behaves on Windows).

But really, you shouldn't be using this really old-school CGI.pm style code anymore. You probably want to look into UP-TO-DATE Comparison of CGI Alternatives, in particular I'm a fan of Mojolicious, its templating engine would be very useful here because it can escape HTML special characters automatically when inserting variables into HTML. I have a bunch of Mojo examples on my scratchpad Update: and I've now posted a full example here.

(Just for the records, I know you know this already! =)

> to help mitigate this is to escape any strings ... HTML::Entities.

Yes escaping mitigates injections, and is a good first workaround.

But I always prefer to strictly reject any unexpected character.

For instance, why should an input "age" include anything else than digits, why "name" more than word characters plus maybe . - and ' ?

Additionally: In my applications I apply the exact same regex filter rules via JS input validators on the client side too.²

Like this I can not only reject broken input on the server side, but also raise an alarm about an intentional manipulation. °

All this of course plus escaping or use of placeholders on all interpreted levels (HTML,JS,Perl,SQL,...).

Security is best provided by multiple lines of defense.

Cheers Rolf
_{(addicted to the Perl Programming Language :)
Wikisyntax for the Monastery}

°) thats part of OWASP too.

²) thankfully, JS copied Perl4 Regex rules almost completely

why "name" more than word characters plus maybe . - and ' ?

See the excellent Falsehoods Programmers Believe About Names. I recognise and applaud your general sentiment but you need to be very careful about rejecting potentially valid data too.

If in doubt, refer to the spec. If there is no spec, insist on one.

---

🦛

why "name" more than word characters plus maybe . - and ' ?

I hope the space between '.' and '-' is significant: it is thankfully rarer these days, but I've lost count of the number of times I was rejected by a web form because "van der Sanden" did not match their concept of a valid surname. In the majority of cases, those websites lost the chance at my custom.

Finding the right balance between strict and permissive can be hard. My preference is to look for unmistakable signals of intentional manipulation - and then ideally blackhole the IP address without further ado - but to accept anything below that high threshold, and concentrate on doing the right thing with it thereafter.

Wow - yes I deleted the whole @pairs codebit and it still worked. I,m a total nitwit with coding so thanx - I,m just playing around and trying things out. The html-injection/cross scripting thing seems dangerous indeed, so okay - I learned its possible to "escape characters" - I would like to do that - do you have a suggestion (example code) for that?

> do you have a suggestion (example code) for that?

the synopsis of HTML::Entities has one

Cheers Rolf
_{(addicted to the Perl Programming Language :)
Wikisyntax for the Monastery}

I'm just playing around and trying things out.

I hope you are aware that running CGI programs in the internet is not a good idea for trying things out. The internet as a whole is not a particularly friendly place. When a new server starts to respond on Port 80, it takes just some minutes before robots will start poking it for security holes. If you point your friends to use your new playground, you're putting them at risk because their browser might run malicious code.

Oh, among other issues, there are security concerns with this code.

• First, the handling of @pairs and $buffer seems to be a leftover from some pre-CGI variation, the %FORM hash is not used in the rest of the code.
• Your code is just appending stuff to the drivers.html file, so this file will probably never be correct HTML - the closing elements are missing.
• As already written by LanX: You accept anything your script gets as parameters and write it to the file. As a harmless example, let a user provide nick=<div style="display:none">. Whatever this user, and all subsequent users, add to the file, will be invisible - until another request happens to close the div element. This is called HTML injection.
• With HTML injection, users can also inject script elements, introducing JavaScript code which is then executed in the context of the browser of whoever visits the script. This code could make use of the fact that another user is logged in in a forum site like PerlMonks, and then let that other user's browser write an article to the forum. The user doesn't even notice what the browser does.
• If said script ends with reloading the same page again, then your users are in an endless loop and sooner or later your web server will break.

You should be aware that the URL leading to your script doesn't need to be called by a browser. A malicious user could easily use LWP::UserAgent or a similar module to feed any complicated stuff into your script.

And yes, all these things have happened a lot of times. The OWASP top ten always lists "Injection" as a prominent security risk.

As a minimum security guard you should prevent user-provided HTML from being processed by the browser by using HTML::Entities or HTML::Escape to encode unsafe characters.

One major rule is to mistrust any input and to filter to a minimal whitelist of allowed/expected characters.

see also

• -T taint ff
• perlsec
• OWASP
• HTML injection

update

after wondering what $buffer means I realized that you don't even use strict and warnings ... 🤷🏽🤦🤷🏽

Security aside

open(my $fh, '>>', 'drivers.html');
[download]

Why not have open tell you why something failed? ...or die "File append failure: $!";

print "Content-type:text/html\r\n\r\n";
[download]

Why are you adding this for every entry? There should be a single Content-Type header. HTML files don't need a content type header, but should be valid, what you're writing isn't.

It'd make a lot more sense to store these values in a database, and display using a template. It looks like you're getting started, I'd suggest not using CGI, if you want to make web development fun and avoid footguns I'd suggest looking at Mojolicious::Lite (Tutorial) with DBD::SQLite, also CGI::Alternatives.

I think you might be following a really outdated CGI tutorial. Here is an example using the much more modern Mojolicious and Mojo::SQLite. It avoids many of the issues mentioned in this thread, but note it is not complete yet, for example there is no checking of duplicate submissions or rate limiting (someone could fill up your hard drive with lots of requests), no constraints on the database table (like unique nicknames), and no checks on the format of the submitted data, like no strict checking of URLs (so somebody could submit an image with nefarious content, or link to a nefarious website, for example). This particular Mojo app works as a CGI script as well, in addition to the usual Mojo ways like development via morbo. I hope this, along with Mojolicious::Guides::Tutorial, perlintro, and perlreftut, will be a much better starting point for you. The script does contain some advanced techniques, like my use of map to work with the columns, but if you have any questions, please feel free to ask!

use Mojolicious::Lite -signatures; # turns on strict and warnings too
use Mojo::SQLite;
use Hash::Ordered;

helper sqlite =>
    sub { state $sql = Mojo::SQLite->new('sqlite:/path/to/mydb.db') };

# disable template cache when running under morbo/development mode
app->renderer->cache->max_keys(0) if app->mode eq 'development';

# define the database columns; used everywhere in this script
# and most code depends on this hash being ordered
tie my %COLS, 'Hash::Ordered',
    nick=>'Nickname', pic=>'Picture', say=>'Says', likes=>'Likes',
    fav=>'Favorite vehicle', car=>'Real life car/vehicle', age=>'Age',
    town=>'Hometown', drink=>'Favorite drink', wpage=>'Webpage';

# dynamically generate the table definition based on the columns
# (if columns are modified later this might mess things up)
my $CREATE = app->sqlite->abstract->generate('create table',
    'drivers', [ 'id INTEGER PRIMARY KEY AUTOINCREMENT',
        map {"$_ TEXT"} sort keys %COLS ] );
app->sqlite->migrations->from_string(<<"END_MIGR")->migrate;
-- 1 up
$CREATE;
-- 1 down
DROP TABLE IF EXISTS drivers;
END_MIGR

# a normal GET request queries the database and renders the template
get '/' => sub ($c) {
    $c->stash( COLS => \%COLS, rows =>
        $c->sqlite->db->select('drivers', [keys %COLS])->hashes );
    $c->render('main');
} => 'drivers';

# a POST request adds a row to the database and redirects back
post '/' => sub ($c) {
    $c->sqlite->db->insert('drivers',
        { map { $_=>$c->param($_) } keys %COLS } );
    $c->redirect_to($c->url_for('drivers'));
} => 'newdriver';

app->start;

__DATA__

@@ main.html.ep
<!DOCTYPE html>
<html>
<head><title>Drivers</title></head>
<body>
<h1>Drivers</h1>

% if (@$rows) {
    <div><table border="1">
    <tr>
    % for my $col (values %$COLS) {
        <th><%= $col %></th>
    % }
    </tr>
    % for my $row (@$rows) {
        <tr>
        % for my $col (keys %$COLS) {
            <td>
            % if ($col eq 'pic') {
                %= image $row->{$col}
            % } elsif ($col eq 'wpage') {
                %= link_to $row->{$col} => $row->{$col}
            % } else {
                %= $row->{$col}
            % }
            </td>
        % }
        </tr>
    % }
    </table></div>
% } else {
    <div><b>No records found!</b></div>
% }

<h2>Add a New Driver</h2>
<div>
%= form_for newdriver => ( method => 'post' ) => begin
<table>
% for my $col (keys %$COLS) {
<tr>
    <td><%= label_for $col => $COLS->{$col} %></td>
    <td>
    % if ($col eq 'pic' || $col eq 'wpage') {
        %= url_field $col, placeholder=>'https://example.com/...',
            required=>'required', pattern=>'https://.*'
    % } else {
        %= text_field $col, placeholder=>$COLS->{$col},
            required=>'required'
    % }
    </td>
</tr>
% }
</table>
%= submit_button 'Add Driver'
%= end
</div>

</body>
</html>
[download]

Thank you :-) I will tinker...

for my $key ( $cgi->param ){
    my @values = $cgi->multi_param( $key );
    $cgi->param( $cgi->escapeHTML("@values") );

}
[download]

my $whatever = encode_entities($whatever, '<>&"');
[download]

#!C:\Perl64\site\bin\perl.exe
use warnings;
use HTML::Entities;
use CGI;

my $cgi = CGI->new(); # create new CGI object

{
   ($name, $value) = split(/=/, $pair);
   $value =~ tr/+/ /;
   $value =~ s/%(..)/pack("C", hex($1))/eg;
   $FORM{$name} = $value;
}

my $nick = $cgi->param('nick');
my $pic = $cgi->param('pic');
my $say = $cgi->param('say');
my $likes = $cgi->param('likes');
my $fav = $cgi->param('fav');
my $car = $cgi->param('car');
my $age = $cgi->param('age');
my $town = $cgi->param('town');
my $drink = $cgi->param('drink');
my $wpage = $cgi->param('wpage');

my $nick = encode_entities($nick, '<>&"');
my $pic = encode_entities($pic, '<>&"');
my $say = encode_entities($say, '<>&"');
my $likes = encode_entities($likes, '<>&"');
my $fav = encode_entities($fav, '<>&"');
my $age = encode_entities($age, '<>&"');
my $town = encode_entities($town, '<>&"');
my $drink = encode_entities($say, '<>&"');
my $say = encode_entities($drink, '<>&"');
my $wpage = encode_entities($wpage, '<>&"');

open(my $fh, '>>', 'drivers.html');
print "Content-type:text/html\r\n\r\n";
print $fh "<b>$nick</b><br><img src='$pic' width='250' height='auto' b
+order='2'><br><br>Says <b>$say</b><br>Likes <b>$likes</b><br>Favorite
+ vehicle <b>$fav</b><br> Real life car/vehicle <b>$car</b><br>Age <b>
+$age</b><br>Hometown <b>$town</b><br>Favorite drink <b>$drink</b><br>
+<b><a href='$wpage'>$wpage</a></b><HR color=#008000 SIZE=2>\n";
print "<html><head><meta http-equiv = 'refresh' content = '0; url = dr
+ivers.html' /></head>";
close $fh;
[download]

• Still no taint mode.
• Still no strict.
• Still code which makes no sense at all (eg. split(/=/, $pair); when $pair is undefined) because you have not used strict.
• Still not enough cleaning of the input data.

See also: How do I make sure users can't enter values into a form that causes my CGI script to do bad things?

---

🦛

Thank you very much guys for the help - I removed the whole split/pair segment and it still works like a charm. I will read and tinker some more about making it safer.

my $nick = $cgi->param('nick');
...
my $wpage = $cgi->param('wpage');

my $nick = encode_entities($nick, '<>&"');
...
my $wpage = encode_entities($wpage, '<>&"');
[download]

Not to address any security hole but just to simplify and DRY out the code a bit, you might try one of these untested approaches.

my ($nick, $pic, $say, $likes, $fav, $car, $age, $town, $drink, $wpage
+)
= map { encode_entities($cgi->param($_), '<>&"') }
qw ( nick   pic   say   likes   fav   car   age   town   drink   wpage
+);
[download]

Or else (and better IMHO):

use constant CGI_PARAMS => qw(
    nick pic say likes fav car age town drink wpage
    );

my %param =
    map { $_ => encode_entities($_, '<>&"') }
    map { $cgi->param($_) }
    CGI_PARAMS
    ;

...

print $fh <<"EOHTML";
<b>$param{'nick'}</b><br>
<img src='$param{'pic'}' width='250' height='auto' border='2'><br>
<br>
Says <b>$param{'say'}</b><br>
Likes <b>$param{'likes'}</b><br>
Favorite vehicle <b>$param{'fav'}</b><br>
Real life car/vehicle <b>$param{'car'}</b><br>
Age <b>$param{'age'}</b><br>
Hometown <b>$param{'town'}</b><br>
Favorite drink <b>$param{'drink'}</b><br>
<b><a href='$param{'wpage'}'>$param{'wpage'}</a></b>
<HR color=#008000 SIZE=2>
EOHTML
[download]

---

Give a man a fish:  <%-{-{-{-<

> simplify and DRY out the code a bit,

simpler and DRYer with loop-aliasing ... ;)

use strict;
use warnings;
use HTML::Entities;

# init
my ($nick, $pic, $say, $likes, $fav, $car, $age, $town, $drink, $wpage
+)=
  ("<html>") x10;

# escape
for my $alias ($nick, $pic, $say, $likes, $fav, $car, $age, $town, $dr
+ink, $wpage) {
    encode_entities($alias);
}

# out
print "$nick ... $wpage";
[download]

&lt;html&gt; ... &lt;html&gt;

edit

or just

encode_entities($_)
   for $nick, $pic, $say, $likes, $fav, $car, $age, $town, $drink, $wp
+age;
[download]

Cheers Rolf
_{(addicted to the Perl Programming Language :)
Wikisyntax for the Monastery}

Still possible to inject code because of your incorrect escaping. It's still as vulnerable as ever.

Could I ask for an example of correct escaping?