How would I make this PHP Script SQL Injection Proof?

Question

   <?php
 
$conn = mysql_connect("localhost", "root", "") or die ('Error connecting to MySQL!');
mysql_select_db("aspire");
 
$earnedpoints = false;
$account = $_POST['name'];
$account = mysql_real_escape_string($account);
 
if ($account == "") {
    echo 'Enter an account name!';
    exit();
}
 
$ip = $_SERVER['REMOTE_ADDR'];
$time = time();
 
$query = mysql_query("SELECT *, SUM(`times`) as amount FROM votingrecords WHERE account='$account' OR ip='$ip'");
$lasttime = mysql_fetch_array($query);
$amount = $lasttime['amount'];
$insertnew = false;    
if ($amount == "") {
    $insertnew = true;
}
$timecalc = $time - $lasttime['date'];
if (!$insertnew) {
    if ($timecalc < 21600) {  
        echo ' Hello '. $account .' you have already voted with this account ('. $account .') or IP ('. $ip .') in the last 6 hours!';
        echo ' Last voted on: '. date('M d\, h:i:s A', $lasttime['date']) .'';
        echo '<html>';
        echo '<head>';
        echo '<meta HTTP-EQUIV="REFRESH" content="10; url=/">';
        echo '</head>';
        echo '<body>';
        echo '<br><br>You will be redirected to the main website in 10 seconds.';
        echo '</body>';
        echo '</html>';
        exit();
    } else {                
        $update = mysql_query("UPDATE votingrecords SET account='$account', date='$time', times=times+1 WHERE ip='$ip'");
            if (!$update) {
                $message  = 'Invalid query: ' . mysql_error() . "\n";
                $message .= 'Whole query: ' . $update;
                die($message);
            } else {
                $earnedpoints = true;
            }
        }
} else {
    $success = mysql_query("INSERT INTO votingrecords (`account`, `ip`, `date`, `times`) VALUES ('$account', '$ip', '$time', 1)");
    if (!$success) {
            $message  = 'Invalid query: ' . mysql_error() . "\n";
            $message .= 'Whole query: ' . $success;
            die($message);
    } else {
        $earnedpoints = true;
    }
}
 
 
 
 
if ($earnedpoints) {
    $points = mysql_query("UPDATE accounts SET votepoints = votepoints + 2 WHERE name='$account'");                
    if (!$points) {
 
            $message  = 'Invalid query: ' . mysql_error() . "\n";
            $message .= 'Whole query: ' . $query;
            die($message);
    }
    mysql_close($conn);
    echo '<html>';
    echo '<head>';
    echo '<meta HTTP-EQUIV="REFRESH" content="0; url=http://www.gtop100.com/in.php?site=80994">';
    echo '</head>';
    echo '</html>';
} else {
    echo 'There was an error processing your request.';
    exit();
}
?>

Hey everyone,

I'm very inexperienced with PHP scripting and I've been told that my script is vulnerable to SQL injection? But I'm not sure how I would make it to be SQL injection proof since I'm not much experienced in that field and i'm afraid I might mess up the code.

Could anyone help me with this? I would greatly appreciate it.

Use prepared statements. – Tim Biegeleisen Jun 19 '17 at 03:12 — Tim Biegeleisen, Jun 19 '17 at 03:12
I'm not sure I understand. – John Smith Jun 19 '17 at 03:12 — John Smith, Jun 19 '17 at 03:12

Akins Nazri · Answer 1 · 2017-06-20T06:34:17.940

0

How can I prevent SQL injection in PHP?

Your code is really escaping the input values but mysql_connect is deprecated in PHP 5.5 and totally dropped in PHP 7. Using parametric query is your best option:

You need to firstly open connection, instead of

$conn = mysql_connect("localhost", "root", "") or die ('Error connecting to MySQL!'); mysql_select_db("aspire");

You will open connection like this

$mysqli = new mysqli("localhost", "root", "", "aspire");

Then prepare your query, instead of putting query like this

$query = mysql_query("SELECT *, SUM(`times`) as amount FROM votingrecords WHERE account='$account' OR ip='$ip'");

You will put it like this

$stmt = $mysqli->prepare("SELECT *, SUM(`times`) as amount FROM votingrecords WHERE account='?' OR ip='?'");

This one is a prepared statement, it is not you that will put your query input, it is PHP that will do it for you, all you need to do is to bind your query input with that $stmt like this

$stmt->bind_param("s", $account);
$stmt->bind_param("s", $ip);

You are having two inputs which are $account and $ip, the account and ip are both string which happens to be what the s stands for... you will now execute the statement, like this

$stmt->execute();

And don't forget to close connection that you opened

$stmt->close();

edited Jun 20 '17 at 06:34

answered Jun 19 '17 at 03:38

Akins Nazri

307
3
10

Hey, I appreciate your help and I'm not sure if I did it correctly but I put my code in pastebin since I couldn't paste it here. https://pastebin.com/NbcXGG92 – John Smith Jun 19 '17 at 12:29
So everything with the code is correct except line 17? Do you mean the place where $ip I should change it to "?"? – John Smith Jun 19 '17 at 13:41
I can't really troubleshoot your code right now, but once you can do all these by yourself you just need structured learning package to have full understanding of how the whole the works, please kindly follow this simple tutorial and everything will become as clear as water [link](https://www.w3schools.com/php/php_ref_mysqli.asp) – Akins Nazri Jun 19 '17 at 14:43
sorry! use this link instead [here](https://www.w3schools.com/php/php_mysql_prepared_statements.asp) – Akins Nazri Jun 19 '17 at 15:16
I will try to fix the script again using your link you gave me, but will you help me troubleshoot it after I finish please? – John Smith Jun 19 '17 at 16:38
I will help you, just have to make sure you put effort, learning is more important than those codes – Akins Nazri Jun 20 '17 at 05:33
I tried to do it and here's the result: https://pastebin.com/CX2pua5S and I'm using this form for this script. https://pastebin.com/WRudF57w But the problem is that it's not working with this new script that I've just made but it's working for the previous script that is shown above. I'm not sure what I'm doing wrong here. – John Smith Jun 21 '17 at 14:24
What was the error? – Akins Nazri Jun 22 '17 at 12:39
It said "Enter a account name" which means that it's not properly loading from my new script but it does from my old script. – John Smith Jun 22 '17 at 17:31

score -1 · Answer 2 · answered Jun 19 '17 at 03:17

-1

see this link http://php.net/manual/en/pdo.prepared-statements.php

use prepared statements and stored procedures

answered Jun 19 '17 at 03:17

gray

103
2
4
20

How would I make this PHP Script SQL Injection Proof?

2 Answers2