in reply to Re: Hacker Proofing My First Script
in thread Hacker Proofing My First Script

I've updated my script, how does this look? Did I use the DBI placeholder correctly, will I be immune from SQL injection attacks?

Thanks for everyones help, I couldn't of done it without your support.

#!/usr/bin/perl -wT use DBI; use strict; use CGI; use POSIX 'strftime'; my $q = CGI->new(); my $sth; my $state = $q->param ('state'); my $city = $q->param ('city'); my $locationname = $q->param ('locationname'); my $referencename = $q->param ('referencename'); my $lat_deg = $q->param ('lat_deg'); my $lat_min = $q->param ('lat_min'); my $lat_sec = $q->param ('lat_sec'); my $long_deg = $q->param ('long_deg'); my $long_min = $q->param ('long_min'); my $long_sec = $q->param ('long_sec'); my $xcoord = $q->param ('xcoord'); my $ycoord = $q->param ('ycoord'); my $ttime = $q->param ('ttime'); my $level = $q->param ('level'); my $radar = $q->param ('radar'); my $laser = $q->param ('laser'); my $vascar = $q->param ('vascar'); my $airplane = $q->param ('airplane'); my $photo = $q->param ('photo'); my $roadblock = $q->param ('roadblock'); my $redlight = $q->param ('redlight'); my $unknown = $q->param ('unknown'); my $comments = $q->param ('comments'); my $email = $q->param ('email'); my $name = $q->param ('name'); my $date = strftime("%A, %B %d, %Y at %H:%M:%S", localtime() ); ##Set Radar Technology################################################ +########## # If technology type isn't selected, it needs to be set to false. if (!$q->param ('radar')) {$radar = "false"}; if (!$q->param ('laser')) {$laser = "false"}; if (!$q->param ('vascar')) {$vascar = "false"}; if (!$q->param ('airplane')) {$airplane = "false"}; if (!$q->param ('photo')) {$photo = "false"}; if (!$q->param ('roadblock')) {$roadblock = "false"}; if (!$q->param ('redlight')) {$redlight = "false"}; if (!$q->param ('unknown')) {$unknown = "false"}; ###################################################################### +########## ##Start database connections########################################## +########## my $database = "database"; my $db_server = "localhost"; my $user = "user"; my $password = "password"; ##Connect to database, insert statement, & disconnect ################ +########## my $dbh = DBI->connect("DBI:mysql:$database:$db_server", $user, $passw +ord); my $statement = "INSERT INTO trap1 (state, city, locationname, referen +cename, lat_deg, lat_min, lat_sec, long_deg, long_min, long_sec, xcoo +rd, ycoord, ttime, level, radar, laser, vascar, airplane, photo, road +block, redlight, unknown, comments, email, name, date_added) VALUES ( +?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; $sth = $dbh->prepare($statement) or die "Couldn't prepare the query +: $sth->errstr"; my $rv = $sth->execute($state,$city,$locationname,$referencename,$lat_ +deg,$lat_min,$lat_sec,$long_deg,$long_min,$long_sec,$xcoord,$ycoord,$ +ttime,$level,$radar,$laser,$vascar,$airplane,$photo,$roadblock,$redli +ght,$unknown,$comments,$email,$name,$date) or die "Couldn't execute q +uery: $dbh->errstr"; my $rc = $sth->finish; $rc = $dbh->disconnect; ###################################################################### +########## print "Content-type: text/html\n\n"; print $q->redirect('http://www.site.com/index.html');

Replies are listed 'Best First'.
Re^3: Hacker Proofing My First Script
by tachyon (Chancellor) on Oct 04, 2004 at 05:10 UTC

    Yes that is secure. Still a bit ugly, but secure nonethless. Why declare $sth at the top, miles from where you use it? Why collect the return codes in $rc - you don't *do* anything with them? It is much more convenient to put constants like your DB passwd etc at the top so you can find them.

    You have a bug. The $q->redirect header will never be acted upon as you terminate the headers with your Content-Type header. You don't need that header, just the redirect. Also print $q->header() will give you a valid header so if you are using CGI why not use it. <code>

    cheers

    tachyon

      I use the $rc return code because I got it as an example out of a book. This is my first DB script so I'm still getting a handle on it.

      Thanks Adam

        I realised that, the point was simply to help your programming thinking, which is basically I am doing this that way because.....

        Have fun with it.

        cheers

        tachyon