Secure coding

PHP Lab: Analyze the code and spot the vulnerability

Introduction and background:

A penetration test has been conducted on the following URL, and a SQL Injection vulnerability was identified.

http://192.168.56.101/webapps/sqli/sqli.php

Learn Secure Coding

Build your secure coding skills in C/C++, iOS, Java, .NET, Node.js, PHP and other languages.

Learn More

The developers were notified about the vulnerability, and they were asked to fix the vulnerability.

After fixing the vulnerability, the new code has been promoted to the following URL.

http://192.168.56.101/webapps/sqli/sqliv2.php

Analyze the fixed code and check if the vulnerability is properly fixed. If not, spot the vulnerability.

Source code has been provided for both original code and fixed code.

The original code can be accessed from:

http://192.168.56.101/webapps/sqli/original.txt

The fixed code for your review can be accessed from

http://192.168.56.101/webapps/sqli/review.txt

Lets begin

Let us begin with reviewing what is reported. Following is the vulnerable code.

<!DOCTYPE html>

<html>

<body>

</form>

</br>

</body>

</html>

<?php

$servername = "localhost";

$username = "root";

$password = "toor";

$dbname = "infosec";

// Create connection

$conn = mysqli_connect($servername, $username, $password, $dbname);

// Check connection

if (!$conn) {

die("Connection failed: " . mysqli_connect_error());

}

$id = $_GET['id'];

$sql = "SELECT * FROM products where ID=".$id;

$result = mysqli_query($conn, $sql);

if (mysqli_num_rows($result) > 0) {

// output data of each row

while($row = mysqli_fetch_assoc($result)) {

echo "ID: " . $row["ID"]. " - Name: " . $row["name"]. " - Price: " . $row["price"]. "<br>";

}

} else {

echo "0 results";

}

mysqli_close($conn);

As highlighted in red, the application takes user input and concatenates it to the existing query to form the final query. If you notice, there is no sanitization performed on the user input thus leaving the application at serious risk.

Now, let us review the code fixed b the developers.

Fixed Code for review:

<!DOCTYPE html>

<html>

<body>

</form>

</br>

</body>

</html>

<?php

$servername = "localhost";

$username = "root";

$password = "toor";

$dbname = "infosec";

// Create connection

$conn = mysqli_connect($servername, $username, $password, $dbname);

// Check connection

if (!$conn) {

die("Connection failed: " . mysqli_connect_error());

}

$id = $_GET['id'];

$id = mysqli_real_escape_string($conn,$id);

$sql = "SELECT * FROM products where ID=".$id;

$result = mysqli_query($conn, $sql);

if (mysqli_num_rows($result) > 0) {

// output data of each row

while($row = mysqli_fetch_assoc($result)) {

echo "ID: " . $row["ID"]. " - Name: " . $row["name"]. " - Price: " . $row["price"]. "<br>";

}

} else {

echo "0 results";

}

mysqli_close($conn);

Does this code look OK? Hmm, no.

As we discussed earlier, mysqli_real_escape_string is used to escape strings and prevent SQL injection on string variables.

Numeric variables are not protected and can be exploited for SQL injection even when passed to mysqli_real_escape_string.

It may look strange for some, but that is the reality.

Just to confirm that the fix provided is not a proper fix, let us run sqlmap once again on the URL where input goes through msqli_real_escape_string.

Let us run the following command.

sqlmap –u "http://192.168.56.101/webapps/sqli/sqliv2.php?id=1 " –D webservice –T users –dump

As expected, it has dumped the table contents from infosec.users.

Fixing SQL Injection - The right way:

The right way to fix SQL Injection is to use prepared statements. This looks as shown in the following code.

<!DOCTYPE html>

<html>

<body>

</form>

</br>

</body>

</html>

<?php

$servername = "localhost";

$username = "root";

$password = "toor";

$dbname = "infosec";

// Create connection

$conn = mysqli_connect($servername, $username, $password, $dbname);

// Check connection

if (!$conn) {

die("Connection failed: " . mysqli_connect_error());

}

$id = $_GET['id'];

$stmt = $conn->prepare( "SELECT * FROM products where ID=?");

$stmt->bind_param("i",$id);

$stmt->execute();

$stmt->store_result();

$num_of_rows = $stmt->num_rows;

$stmt->bind_result($id,$name,$price);

$result = mysqli_query($conn, $sql);

if ($num_of_rows > 0) {

// output data of each row

while($stmt->fetch()) {

echo "ID: " . $id. " - Name: " . $name. " - Price: " . $price. "<br>";

}

$stmt->close();

} else {

echo "0 results";

}

mysqli_close($conn);

As highlighted in red, we are defining the SQL code that is to be executed with placeholders (?) for parameter values, programmatically adding the parameter values, and then executing the query. Doing this will prevent any "injected" SQL from being executed as we are properly casting the input to the right data type.

In our case, the following line shows how we are casting the input taken into an integer using "I."

$stmt->bind_param("i",$id);

Final fixed code can be accessed from

http://192.168.56.101/webapps/sqli/fixed.txt

The application, which is fixed can be accessed from the following URL

http://192.168.56.101/webapps/sqli/sqlifixed.php

The following figure shows the payloads working earlier not working anymore.

You may try to run SQLMAP on this URL to see if it is still exploitable. You will see the following error showing that SQLMAP failed.

Learn Secure Coding

Build your secure coding skills in C/C++, iOS, Java, .NET, Node.js, PHP and other languages.

Learn More

Posted: August 16, 2017

Srinivas

View Profile

Srinivas is an Information Security professional with 4 years of industry experience in Web, Mobile and Infrastructure Penetration Testing. He is currently a security researcher at Infosec Institute Inc. He holds Offensive Security Certified Professional(OSCP) Certification. He blogs atwww.androidpentesting.com. Email: srini0x00@gmail.com