2

I am creating a php page with a small and simple database.

when I visit it online and try to pass the parameter "length" in the url like: index.php/?length=1 it works fine and fetches the data.

If I add the single quote like index.php/?length=1' I have no SQL error on the page...

but if I use index.php/?length=-1 I see the SQL error in my page.

Does this mean that my page is vulnerable?

How can I further test it and fix the problem?

Edit: added the code 

$length = $wpdb->get_results( $wpdb->prepare("SELECT `title`, `website`, `material`, `color`, `width`, `height`, `group`, `category`, `numbers_positive`, `numbers_negative`, `custom` FROM {$wpdb->shirts} WHERE `id` = '%d' ORDER BY `rank` ASC, `id` ASC", intval($shirt_id)) );

if (!isset($shirt[0])) return false;

$shirt= $shirt[0];
$shirt->title = htmlspecialchars(stripslashes($shirt->title), ENT_QUOTES);
$shirt->custom = maybe_unserialize($shirt->custom);
$shirt->color = maybe_unserialize($shirt->color);
if ( $this->hasBridge() ) {
    global $lmBridge;
    $shirt->shirtColor = $lmBridge->getShirtColor($shirt->color);
}
$shirt = (object)array_merge((array)$shirt,(array)$shirt->custom);
unset($shirt->custom);

return $shirt;
Alberto Martinez
  • 2,620
  • 4
  • 25
  • 28
qefseri
  • 21
  • 2

2 Answers2

1

Yes, from the URL examples you have given, it seems like you take user input and directly insert it into your MySQL statement. That is the absolute worst. You should always parse user input because direct input from a user can result in the string being escaped and them deleting every table in your DB. This is a great example: Bobby Tables

Also, this is been a topic of great discussion. There is a great answer here

Edit* Using the WordPress framework and looking at your code, its not as bad as it seemed.

Ice76
  • 1,143
  • 8
  • 16
  • Thank you for getting back at me! Please be patient because I am trying to learn and understand. This is the code: https://pastebin.com/sXhtPG7r Where might the problem be here in this piece of code? If there really is a problem, based on this code how should I try and inject the url? because the basic tests I made didn't have any issue. Thanks again – qefseri Sep 01 '17 at 23:15
  • Ahh, your using prepared statements in WordPress. That does make it a lot safer because the framework handles a lot of security checks for you. The SQL error you see is probably because id -1 (negative 1) does not exist and `intval` will still return a negative number. The quote being added gets removed with `intval` – Ice76 Sep 02 '17 at 00:04
0

accepting, but generating an error on -1 does not nessicarily mean you are suseptable to an injection attack. As long as you are varifying that the input is an integer and only using the integer compontent, you're fairly safe.

Prepared statements make it even more secure, by seperating the data from the query. Doing that means someone can never 'break out' of what you are supposed to be working on. It's absolutely the right way to use SQL.

We can even take it another step farther, by limiting the abilty of the account to do anything other that run stored queries, and storing your queries on the SQL server side, rather then in your PHP. At that point, even IF they broke out (which they can't), they would only be able to access those defined queries.

A Hettinger
  • 448
  • 2
  • 11