-1

I'm writing you since I have been told that the code written below is unsecured because I can be hacked through SQL Injection.

I have tried to read some guides related to the "prepared statement" topic but I did not manage to solve the issue.
Can you please help me out? I would like to understand and solve my issue.
Thank you in advance to everyone wishing to help.

<html>
<body>
<form action='index.php' method='post'>
<h2>Select Departure:</h2>
<select name="departures" class="form-control">
        <option value="">--- Select Departure ---</option>

                    <?php
                        require('prova1.php');
                        $sql1 = "SELECT * FROM departures"; 
                        $sql2 = "SELECT * FROM arrivals"; 
                        $result1 = $mysqli->query($sql1);
                        while($row1 = $result1->fetch_assoc()){
                    ?>   
                    <option value="<?php echo $row1["dep_name"]; ?>"><?php echo $row1["dep_name"]; ?></option>
                    <?php } ?>

</select>
<br>

<h2>Select Arrival:</h2>
<select name="arrivals" class="form-control">
        <option value="">--- Select Arrival ---</option>

                    <?php
                        $result2 = $mysqli->query($sql2);
                        while($row2 = $result2->fetch_assoc()){
                    ?>   
                    <option value="<?php echo $row2["arr_name"]; ?>"><?php echo $row2["arr_name"]; ?></option>
                    <?php } ?>

</select>
<br>
<h2>Select # of passengers</h2>
<select name="passengers" class="form-control">
        <option value="">--- # of passengers ---</option>
        <option value="1">1</option>
        <option value="2">2</option>
        <option value="3">3</option>
        <option value="4">4</option>
        <option value="5">5</option>
        <option value="6">6</option>
        <option value="7">7</option>
</select>
<br>

<h2>Select # of bags</h2>
<select name="bags" class="form-control">
        <option value="">--- # of bags ---</option>
        <option value="1">1</option>
        <option value="2">2</option>
        <option value="3">3</option>
        <option value="4">4</option>
        <option value="5">5</option>
        <option value="6">6</option>
        <option value="7">7</option>
</select>
<br>

<input type='submit' name='submit' id='submit' value='Get Selected Values' />
</form>

<?php
    if(isset($_POST['submit'])){
    $selected_val1 = $_POST['departures'];  
    $selected_val2 = $_POST['arrivals'];
    $selected_val3 = $_POST['passengers'];
    $selected_val4 = $_POST['bags'];    
    if ($selected_val3 < 4 AND $selected_val4 < 4){
    echo "You will drive with a taxi!"; 
    $query3 = "SELECT * FROM taxilist WHERE dep_name = '".$selected_val1."' AND arr_name = '".$selected_val2."'";
    } else {
    echo "You will drive with a van!";
    $query3 = "SELECT * FROM vanlist WHERE dep_name = '".$selected_val1."' AND arr_name = '".$selected_val2."'";
    }
    require('prova1.php');
    echo "<br>The price from " .$selected_val1. " to " .$selected_val2. " is: ";
    $result3 = $mysqli->query($query3);
        while($row3 = $result3->fetch_assoc()){
        echo $row3['price'];
        }
    }
?>
</body>
</html>
Al.G.
  • 4,327
  • 6
  • 31
  • 56
Melinda S.
  • 1
  • 1
  • 3

2 Answers2

3

The problem is that your code is including user-provided data in follow-up SQL statements:

$selected_val1 = $_POST['departures'];  
$query3 = "SELECT * FROM taxilist WHERE dep_name = '".$selected_val1."' AND arr_name = '".$selected_val2."'";

When you construct SQL strings this way there is no way to make sure that you properly separate the commands (SELECT ... FROM) and the data ( in the SQL statement. By carefully constructing the data a malicious user may gain control of the details of the command being sent to the database. In this case it means data from any table in this database might be leaked.

Example: An attacker could send the value ' UNION SELECT id, username, password FROM users -- as the POST variable departures which would result in the following SQL query being sent to your database: SELECT * FROM taxilist WHERE dep_name = '' UNION JOIN SELECT id,username,password FROM users -- '. Assuming the table users exist and that taxilist returns 3 columns by default, the entire contents of the users table is now included in the output. Tools like SQLMap will find these vulnerabilities in seconds.

You can fix this by using prepared statements or by properly escaping the user provided data so that the data cannot become part of the command. Using something like this would would patch up this issue:

$selected_val1 = $mysqli->real_escape_string($_POST['departures']);
$selected_val2 = $mysqli->real_escape_string($_POST['arrivals']); 
$query3 = "SELECT * FROM taxilist WHERE dep_name = '".$selected_val1."' AND arr_name = '".$selected_val2."'";

The example attack would now result in the following query: SELECT * FROM taxilist WHERE dep_name = '\' UNION JOIN SELECT id,username,password FROM users --'. It would just be looking for a weird dep_name now instead of revealing your users table.

NSSec
  • 4,431
  • 1
  • 27
  • 29
  • Thank you! I have tried what you said but I get the following error: "Uncaught Error: Call to undefined function real_escape_string() in C:\xampp\htdocs\test\index.php:69 Stack trace: #0 {main}". Do you know how to solve it? – Melinda S. Oct 01 '17 at 15:58
  • @MelindaS, What exactly did you try? It sounds like you really shouldn't be messing with this and should leave it to a professional... – Mat Oct 01 '17 at 21:06
  • @NSSec Forgot to tag you! See above – Melinda S. Oct 01 '17 at 21:12
  • @MatthewB I tried to use real_escape_string() – Melinda S. Oct 01 '17 at 21:13
  • It seems you are trying to call a function real_escape_string() as opposed to calling the method real_escape_string() on the $mysqli object. – NSSec Oct 01 '17 at 21:33
  • @NSSec, in the `example` I really think you forgot to put the word `JOIN` in the send message from the attacker – Yuri Aps Nov 21 '22 at 21:42
0

Try this:
https://www.w3schools.com/sql/sql_injection.asp

It's from W3Schools so they probably know what they're talking about.

Jacob Goddard
  • 17
  • 1
  • 3
  • 1
    I did not down vote but in general Stack Overflow does not like it when you just give a link. Links are fine, just use them to supplement your answer, which should answer the question on its own. This is written in https://stackoverflow.com/help/how-to-answer – Bill Karwin Oct 06 '17 at 16:24