Help with SQL injection

Hey guys,

First of all, I know this code is really unorganised and that mysql functions are deprecated and should not be used, however I’m working for someone on a very old website and I need to check that the code protects against SQL injection.

Normally I would use PDO and prepared statements but again its an old website and I’m just using mysql_real_escape_string which apparently is used to protect against SQL injection however I’ve not used it before so I don’t know for certain.

The script is used on an admin panel to make a new php page. Again, I would normally use a different approach but it just has to be done this way. The code creates a page using fopen and fwrite then on the page it gets the content from the database however the SQL query didn’t work without the $value variable being wrapped in quotes so I wrote a quick function to do this.

It uses the $_SERVER[‘REQUEST_URI’] value e.g. the file name to select the information from the database and the $value variable is being escaped using mysql_real_escape_string before being passed to the query.

Is the code secure and will it stop SQL injection? Again I know it’s a mess, you should see the rest of the site, it’s a disaster.

Thanks,
Adam

[php]
function quotes($string) {
return “’” . $string . “’”;
}

function escapeString($data){
return mysql_real_escape_string($data,$this->curcon);
}

$content = '<?php
include_once “header.php”;
include_once “menu.php”;
$url = parse_url($_SERVER[“REQUEST_URI”]);
$path = str_replace("/","",$url[“path”]);
$value = $db->escapeString($path);
$sql = mysql_query("SELECT * FROM static_pages WHERE url = " . quotes($value));
$results = mysql_fetch_assoc($sql);
?>

<?php echo stripslashes($results["pageTitle"])?>

<?php echo stripslashes($results["pageContent"]); ?>
<?php include_once "right-sidebar.php"; ?>
<?php include_once "footer.php"; ?> ';

$file = fopen(’…/’ . $_POST[“url”],“w”);
$write = fwrite($file,$content);
$close_file = fclose($file);
[/php]

There is a reason it was removed. So, saying that is secure would be a fallacy, it is more secure than without, but if it actually did anything useful, they wouldn’t have changed how it worked in the first place.

mysql_real_escape_string is not a silver bullet against SQLi, it depends on the context/query.

On the same note, stripslashes is not suitable to protect against XSS.

Are you sure you want your name affiliated with this project?

None of your request even matters. In the current release of Php the code will flat out not work at all no matter what you do. If they wont have you write it to current standards, walk away. You will not be doing anyone any good.

The [tt]mysql_real_escape_string()[/tt] function is fine if you always(!) wrap the result in quotes (you should include that in your escape function) and make sure that you don’t fudge up the character encoding. What I mean by encoding is this: Many people use a [tt]SET NAMES[/tt] query to change the character encoding of the database connection (e. g. from the default Latin 1 to UTF-8), but this is fatal for functions like [tt]mysql_real_escape_string()[/tt], because PHP isn’t aware of the change and will assume the wrong encoding. In the worst case, [tt]mysql_real_escape_string()[/tt] will no longer recognize critical characters. If you need to change the encoding, always use [tt]mysql_set_charset[/tt].

Besides those two pitfalls, there’s nothing wrong with the function.

Of course we all agree that you’re riding a dead horse, and since the code hasn’t been updated for more than a decade, it probably has security vulnerabilities all over the place (like the XSS vulnerabilites pointed out by JimL). Also note that the old ext/mysql extension has actually been removed in the PHP 7 branch, so once PHP 5 reaches end of life (in less than two years), the website in it’s current form will definitely be dead.

[member=76351]Pretty Homepages[/member], Nice informative response. I would soak it into my head but it is pointless since I will never be using deprecated code.

Sponsor our Newsletter | Privacy Policy | Terms of Service