SQL injection?

[callumacrae]

Hi,

http://paperhub.x10hosting.com/links.php

I have been told that it would be quite easy to do an SQL injection on this page.

Does anyone know if this is true, and if it is how would I fix it?

~Callum

 

[marshian]

I tried a couple of possible exploits, but I don't think any of them worked (however, I could be wrong too). What you should pay attention for sql injections to is every variable that goes in your query. For example with the order by: if $_GET["o"] is not equal to "asc" or "desc", don't use the value in your query. Pay special attention to strings that are actually dependant on user input, such as usernames. If such variables are used in your query, ALWAYS use mysql_escape_string on them, to make sure all special characters in sql are escaped.

Example:

$query = "SELECT * FROM `users` WHERE `login`= '".mysql_escape_string($_GET["login"])."'";

 

[descalzo]

I have been told that it would be quite easy to do an SQL injection on this page.

Does anyone know if this is true, and if it is how would I fix it?

~Callum

The front end (ie the page with the fields) tells you nothing about the security of the script.

As marshian points out, never trust input submitted over the web. Check it for unsafe characters. Check it for length. Check it for acceptable values.

 

[slacker3]

(visited only http://paperhub.x10hosting.com/links.php )

My suggestion would be to use POST instead of GET in every form, and to use pre-fabricated query's (if possible).

eg.

if ( POST_ob == name && POST_o == asc)
   // query, no variables in here
else
if ( POST_ob == added && POST_o == desc)
   // query, no variables in here
else
...

(ugly pseudo code)

escape every user input (it couldn't hurt to escape your output, too..)


easy and cheap solution

 

[misson]

Better yet, use a DB driver (such as PDO) that supports prepared statements, which aren't vulnerable to SQL injection. You still have to consider other types of injection attacks, such as XSS.

 

[vishal]

My suggestion ,WE prevent SQL injections as opposed to binary, is to use URL Encoding or Hex Encoding.
I haven't seen a complete example of stopping SQL Injections, most refer to use the mysql_real_escape_string function or param statements.

 

[callumacrae]

So is this better:

<?php
if(isset($_GET['ob']) && isset($_GET['o']) && (($_GET['o'] == "asc") || ($_GET['o'] == "desc")) && (($_GET['ob'] == "name") || ($_GET['ob'] == "added") || ($_GET['ob'] == "date")))
{
$sql = "SELECT * FROM `links` ORDER BY `{$_GET['ob']}` {$_GET['o']}";
}else{
$sql = "SELECT * FROM `links` ORDER BY `Name` ASC";
}
$result = mysqli_query($cxn,$sql) or die("SQL failed");
$num = 1;
while ($row = mysqli_fetch_array($result))
{
extract($row);
echo "<a href=\"$Link\"><b>$Name</b></a><br /><p>$Description</p><p style=\"font-size:7pt\">Added by $added at $Date</p><br />";
$num++;
} 
?>

Thanks for helping

~Callum

 

[marshian]

Yes, that code cannot create a query with any values different than legit ones. Very good
Make sure your search function is as safe as this one and you'll be sql-injection-free.

 

[callumacrae]

My search boxes are written by google and phpbb, so I'm kinda hoping they're secure

Thanks again,
~Callum

 

[slacker3]

My search boxes are written by google and phpbb, so I'm kinda hoping they're secure

Thanks again,
~Callum

They should be fine.

You can test the most common SQL(or XSS)-injection attacks against your forms automatically
with the firefox-extension "SQL Inject-Me" (or "XSS-me") which is part of the "Exploit-Me" Suite.

List of useful Firefox Extensions on my site. (the last one in "Security auditing")

 

[callumacrae]

Couldn't that be abused? :/

~Callum

 

[Twinkie]

I don't mean to hijack your thread (I think the topics are related), but what attacks can be launched against a site with magic quotes enabled on the server?

 

[vishal]

Twinkie canu may the above concept of magic quotes a little clear

 

[marshian]

Twinkie canu may the above concept of magic quotes a little clear

http://be2.php.net/magic_quotes

@Twinkie:
Using magic quotes can cause a problem in some contexts. You might want to store data as it is given, not with quotes escaped. Therefore, it's possible the programmer unescapes toe quotes in order to use them, rendering magic quotes a waste of time.
Secondly, I don't think magic quotes escapes single quotes ( ' ), mysql-style quotes ( ` ), the number sign ( # ), semicolons ( ; ), ...
And those "happen" to be important characters in SQL queries...

 

[vishal]

marshian what you said is a pont to be noted, good
pls try to share more about it

 

[descalzo]

Secondly, I don't think magic quotes escapes single quotes ( ' ), mysql-style quotes ( ` ), the number sign ( # ), semicolons ( ; ), ...
And those "happen" to be important characters in SQL queries...

What magic quotes does, from the PHP manual:

When on, all '(single-quote), " (double quote), / (backslash) and NULL characters are escaped with a backslash automatically. This is identical to what addslashes() does.

 

[marshian]

What magic quotes does, from the PHP manual:

Hmm, ok, but that still leaves ` ; and #, and those are only examples, there's more where that came from. But indeed, single quotes are important.
But then, returning to the original question, it's again easy to abuse it: magic quotes doesn't do it SQL-style. If you use a magic-quoted string in a SQL query, it's still unsafe due to the fact single quotes in SQL aren't quoted with a backslash, but with a second single quote!

PS:

When on, all '(single-quote), " (double quote), / (backslash) and NULL characters are escaped with a backslash automatically. This is identical to what addslashes() does.

That's not a backslash, this is a backslash: \ :dunno: Don't we all like well-written manuals?

 

[nosunshine]

I can confirm that your site is vulnerable to an attack on the link mod, via an exploit posted on milw0rm.

Be sure to fix this soon. Upgrading the links mod ought to fix it. You can always run the milw0rm script yourself to check against any changes you make.

 

[misson]

So is this better:

<?php
if(isset($_GET['ob']) && isset($_GET['o']) && (($_GET['o'] == "asc") || ($_GET['o'] == "desc")) && (($_GET['ob'] == "name") || ($_GET['ob'] == "added") || ($_GET['ob'] == "date")))
{
   ...

Safe but neither efficient nor readable nor scalable. You can make use of associative arrays to address these issues. Wrap the sanitization in a function so you can reuse it:

function sanitize(&$input, $whitelist, $defaults) {
    foreach ($defaults as $key => $dflt) {
        if (! isset($input[$key]) || !isset($whitelist[$key][strtolower($input[$key])])) {
            $input[$key] = $dflt;
        } else {
            // put value in canonical form
            $input[$key] = $whitelist[$key][strtolower($input[$key])];
        }
    }
    return $input;
}

Usage is simple:

$searchWhitelist = array('ob' => array('name' => 'Name', 'added' => 'Added', 'date' => 'Date'),
                         'o' => array('asc' => 'ASC', 'desc' => 'DESC'));
$searchDefaults = array('ob' => 'Name', 'o' => 'ASC');

sanitize($_GET, $searchWhitelist, $searchDefaults);
$sql = "SELECT * FROM `links` ORDER BY `{$_GET['ob']}` {$_GET['o']}";
...

This implementation is case-insensitive, so query strings such as "ob=DATE" will work.

As it stands, the code (yours and mine) leaks information about database fields through the input field values (visitors can discern that the table has fields `Name`, `Added` and `Date`). While this isn't a huge security concern, it isn't best practice. One neat thing about sanitize() is it easily lets you divorce input field values from the values you use in the queries; all you need to do is change the keys in the whitelist inner arrays. For example:

$searchWhitelist = array('ob' => array('Name', 'Added', 'Date'),
                         'o' => array('ASC', 'DESC'));

This will use numeric values; "ob=1&o=1" will sort by Added in descending order.

$searchWhitelist = array('ob' => array('n' => 'Name', 'a' => 'Added', 'd' => 'Date'),
                         'o' => array('a' => 'ASC', 'd' => 'DESC'));

This uses single-letter values; "ob=a&o=d" will sort by Added in descending order.

 

[callumacrae]

Thanks

What's milw0rm?

~Callum