3
\$\begingroup\$

I'm deploying a PHP project, and I always try to use prepared statements, but unfortunately, GoDaddy does not have the mod mysqlnd which I use it for the function mysqli_stmt_get_result($stmt), so I had to start using mysqli_query in order to use the classic.

Example:

while($row = mysqli_fetch_array($query,MYSQLI_ASSOC)){
   echo $row['Name'];
}

I've been using mysqli_query, but I'm not expert in security, so could you please evaluate or give me some tips?

<?php 
   include '../session.php';
   include '../connection.php';
   $link = mysqli_real_escape_string($connection,$_GET['file']);
   $query = mysqli_query($connection,'CALL mostrarConvocatoriaLink("'.$link.'")');
   if(!$query){
     echo mysqli_error($connection);
   }
   else{
     $row = mysqli_fetch_array($query,MYSQLI_BOTH);
     extract($row);
   }
   mysqli_close($connection);
?>

This is used to get an entry on my database using a procedure (which is only a Select statement) but, we're using a random generated string to create the links, instead of using the ID of the page. This is for using a kind of a WordPress that we're creating for the project. In my project all the inserts are with prepared statements and using Post, but in this case, I create a <a href="template.php?file=[the random string]">. I wanna know if I'm screwing things up.

\$\endgroup\$
  • \$\begingroup\$ The title of your post should be the function/purpose of your code \$\endgroup\$ – SirPython Mar 14 '15 at 0:21
1
\$\begingroup\$

As a rule of thumb in code security if you don't know what attacks you've secured your code from then it is not secure. At least you're considering that your code might have issues and looking to remedy them when many don't.

Basically consider the ID 7254872364;drop table users;// or some variant - what SQL statement is that going to produce as a string and is it what you thought would get? I should get that will get through real_escape_string quite nicely.

One thing that will help is to rewrite your code to use prepared statements. They are not a magic bullet (nothing is) but they will give you code that is an order of magnitude more secure than you have today.

These questions on Security StackExchange should make for some interesting further reading:-

[Edit: Sorry, missed that your host doesn't provide support for prepared statements. GoDaddy do support it but you have to enable it in the control panel. See Enable MYSQL-PDO in godaddy windows server and if you can't get it solved it might be best to tell GoDaddy where to go]

It's a good practice to assume that all user input is hostile until it you've established otherwise, which doesn't seem to be happening in this code. Sanitize anything that comes from outside your system at every point of entry interaction including those pesky Url parameters, fields, cookies - at some point the odds are that it will be hostile. If the ID's will take a certain form (x digits for example) then validate them and stop processing with an error if that validation fails.

\$\endgroup\$
  • \$\begingroup\$ Yeah i know that they suport them, but the fucntion that does not support is mysqli_stmt_get_result($stmt) so i had to patch it with mysqli_query. I know i know, i have to start using PDO Haha. I need to sit down and get the feel for PDO, and now i'm against time so i have to stick with this for now. Any ideas to try and make it more secure? \$\endgroup\$ – Hector Mendoza Jacobo Nov 13 '14 at 20:42
  • \$\begingroup\$ For the most part what I added in my edit is the major thing from your app, if it doesn't look right then fail the whole request. On top of that you can review the system as a whole look at each component on the basis that your app is compromised... ask what could a bad request do and can you avoid or mitigate it - does a read request need write access (do you need separate read/write auth?) Can php write to the folder? How are the db credentials stored (like httpd.conf in the code?) Ultimately there's not a quick fix to developing secure code, it's something you design in from day 0. \$\endgroup\$ – James Snell Nov 13 '14 at 22:24
  • \$\begingroup\$ @JamesSnell I don't think that your example attack would go through, because you are not closing the ' that starts the string. If used correctly, mysqli_real_escape_string + single quotes is actually considered secure (with the exception of certain multi-byte character sets). It's still not recommended, because it's easy to mess up (for example, single quotes are quite easy to overlook). So I definitely agree, prepared statements are the way to go, even if there is no direct security issue in this code. \$\endgroup\$ – tim Feb 11 '15 at 23:34
  • \$\begingroup\$ sorry, the example actually uses normal quotes, not single quotes (yeah, it's really easy to misread quotes :) ). But those are escaped by mysqli_real_escape_string as well. (Btw, another reason your example will not work is that mysqli_query does not support stacked queries.) \$\endgroup\$ – tim Feb 11 '15 at 23:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.