SQL Injection

[MoD01]

in your inventory management tool, it is possible to do SQL injection(s).

How to reproduce:

• A user with (type=Full Access) is logged in
• Malroy (the attacker) sends the user an specially crafted URI:
  • localhost/itdb/index.php?action=edititem&delid=1 OR 1=1
• The user clicks the malicious URI

➡️ This will delete all items stored in the database!

What is the problem:

• The first problem is, that you invoke actions using a GET requests. This does not send a XSRF-token, thereby an attacker can trick you to execute the delete action by clicking the malicious url.

• The second problem: You do not restrict the delid to be e.g. only a number. This allows an attacker to inject an arbitrary SQL query.

• The third problem: The attackers' SQL query is executed, because you do not use prepared statements.

How to patch this vulnerability:

Replace lines 24-25 (https://github.com/sivann/itdb/blob/master/php/edititem.php#L24) with this:

$sql=$mysqli->prepare("DELETE from item2file where itemid=?"); // ? := placeholder for variable content
$sql->bind_param('i', $delid); // i := corresponding variable has type integer
$sql->execute(); // execute prepared statement
$sql->close(); // close statement and connection

And use mysqli for the db connections instead, e.g.: $mysqli = new mysqli('localhost', 'my_user', 'my_password', 'world'); Here you can find more information/examples : http://php.net/manual/en/mysqli-stmt.bind-param.php

How to fix the problem in general:

• Use prepared statements and parameterized queries for all your SQL! These are SQL statements that are sent to and parsed by the database server separately from any parameters. This way it is impossible for an attacker to inject malicious SQL. See: https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet

• Dont modify/delete/... with GET

Vulnerability found by:

• @stweigand
• @MoD01

[sivann]

Hi, thanks for your analytical report; I know ITDB is not secure, it is aimed for intranets mostly. It was written on a few afternoons a very long time ago, before adding to github. I will add a security warning immediately on the frontpage as I have failed to do so. As stated in the "Welcomed pull requests" It needs a complete rewrite with PDO and prepared statements and a framework. I will not commit those changes as it's a drop in the ocean of security issues. Thank you for your understanding.