Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

Any security holes?

by Limbomusic (Acolyte)
on Jun 26, 2022 at 13:49 UTC ( #11145079=perlquestion: print w/replies, xml ) Need Help??

Limbomusic has asked for the wisdom of the Perl Monks concerning the following question:

I have a simple html form which uses POST to append text to a html-file and I was wondering if there are any security concerns in my .pl file? My .pl-file:
#!C:\Perl64\site\bin\perl.exe use CGI; my $cgi = CGI->new(); # create new CGI object # Split information into name/value pairs @pairs = split(/&/, $buffer); foreach $pair (@pairs) { ($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'); 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;
My webserver runs on windows 10 (with perl strawberry or something) - using hiawatha webserver. And I'm also wondering - when using POST - its possible to paste in whatever in the form - if someone was trying to hack or just mess things up - could that be done and how? And if so, are there any preventive measures I could take?

Replies are listed 'Best First'.
Re: Any security holes?
by haukex (Archbishop) on Jun 26, 2022 at 15:23 UTC
    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?
        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.

Re: Any security holes?
by haj (Priest) on Jun 26, 2022 at 15:48 UTC

    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.

Re: Any security holes?
by LanX (Sage) on Jun 26, 2022 at 13:56 UTC
    I think an attacker could at least inject HTML/JS/XSS into your web-page and damage the user.

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

    see also

    update

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

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

Re: Any security holes?
by marto (Cardinal) on Jun 28, 2022 at 08:57 UTC

    Security aside

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

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

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

    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.

Re: Any security holes?
by haukex (Archbishop) on Jun 28, 2022 at 10:57 UTC

    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>
      Thank you :-) I will tinker...
Re: Any security holes?
by perlfan (Vicar) on Jun 26, 2022 at 15:18 UTC
    Check out CGI::Tiny. Also, why are you writing this to a file and not returning the content to the parent process?
Re: Any security holes?
by Anonymous Monk on Jun 27, 2022 at 10:50 UTC
    Heres cuteness CGI
    for my $key ( $cgi->param ){ my @values = $cgi->multi_param( $key ); $cgi->param( $cgi->escapeHTML("@values") ); }
Re: Any security holes?
by Limbomusic (Acolyte) on Jun 26, 2022 at 23:57 UTC
    All right
    my $whatever = encode_entities($whatever, '<>&"');
    did the trick! Now, if I put html code into the input field it doesnt mess up anything. Thanx for steering me to the answer. (and fast too) Heartily appreciated. Hugs Are there still any blatant security risks? My script now:
    #!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;
        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, '<>&"');

      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 +);
      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


      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";

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

        edit

        or just

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

        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?
Re: Any security holes?
by Limbomusic (Acolyte) on Jun 28, 2022 at 11:53 UTC
    Hm. I coulndt (wouldnt) use encode_entities($_) after all.. because then its not possible to post emojis in the forms... they will just show as weird letters. So I went back to using my $navn = encode_entities($navn, '<>&"'); which at least prevents html-code to mess up the html.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://11145079]
Approved by philipbailey
Front-paged by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others about the Monastery: (1)
As of 2023-06-03 12:08 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    How often do you go to conferences?






    Results (14 votes). Check out past polls.

    Notices?