-2

I've made a simple search-script in PHP that searches a mySQL database and outputs the result. How this works is like this:

  • User searches for "jack's" through a search-form.
  • My PHP-script GETs this search, and sanitizes it.
  • Then the script, with the use of SELECT and LIKE, gets the results.
  • The script then outputs the result to the user.
  • Lastly, the script tells the user that "jack's returned x results." with the help of escaping.

What I would like to ask is, am I doing it right?

This is how I sanitize before SELECTING from the database:

if(isset($_GET['q'])){
  if(strlen(trim($_GET['q'])) >= 2){
    $q = trim(mysql_real_escape_string(addcslashes($_GET['q'], '%_')));
    $sql = "SELECT name, age, address FROM book WHERE name LIKE '%".$q."%'";
  }
}

And this is how I escape before outputting "jack's returned x results.":

echo htmlspecialchars(stripslashes($q)) . " returned x results.";

Is this the correct way to do it?

By the way, I know that PDO and mySQLi is preferred as they sanitize themselves through the use of prepared statements, but I have no real experience with them whatsoever. But I would gladly take a look, if you guys could link me some newbie tutorials/explanations. Furthermore, I heard that magic_quotes and charset could in some way or another lead to injections -- is this correct?

AnonymousJ
  • 87
  • 1
  • 12
  • Your escaping is applied in exactly the wrong order. Maybe explain how you thought it would work. – mario Apr 21 '13 at 17:51
  • @mario It is? I don't know which order works better/is more correct. Let me explain anyway. When the script `GET`s, the input gets trimmed and it checks if the input equals or is more than 2 characters long. Afterwards it sanitizes by first adding slashes to `%` and `_` to prevent wildcard-searching, then it sanitizes the input through the use of `mysql_real_escape_string`, and lastly it trims if there is white space at the beginning or at the end of the input. Whatever it sanitized is now being searched in the database. As for the escaping -- it works, but I got it from a random topic. – AnonymousJ Apr 21 '13 at 18:02
  • Well, trimming goes first, then m_r_e_s, then addcslashes for the LIKE glob. – mario Apr 21 '13 at 18:06
  • Okay. But could you explain like I'm five, how the order of the different sanitization functions is important? I mean.. could you give an example on how `trim, m_r_e_s, addcslashes` works different/better than `addcslashes, m_r_e_s, trim`? – AnonymousJ Apr 21 '13 at 18:13
  • And could you comment on the output? Do I escape it properly? Do I even need the `htmlspecialchars` function? Or could it be replaced with something better? – AnonymousJ Apr 21 '13 at 18:32

1 Answers1

0

For some reason we need also escape a backslash too.
So, the proper code would be, I believe

if(isset($_GET['q'])){
  $_GET['q'] = trim($_GET['q']);
  if(strlen($_GET['q']) >= 2){
    $q = $_GET['q'];
    $q = '%'.addCslashes($q, '\%_').'%';
    // now we have the value ready either for escaping or binding
    $q = mysql_real_escape_string($q);
    $sql = "SELECT name, age, address FROM book WHERE name LIKE '$q'";
    //or 
    $sql = "SELECT name, age, address FROM book WHERE name LIKE ?";
    $stm = $pdo->prepare($sql);
    $stm->execute(array($q));
    $data = $stm->fetchAll();
  }
}

For the output, use 

echo htmlspecialchars($_GET['q']);

stripslashes not needed here.

Furthermore, I heard that magic_quotes and charset could in some way or another lead to injections -- is this correct?

magic quotes won't harm your security if you won't use them.
charset is dangerous in case of some extremely rare encodings but only if improperly set. if mysql(i)_set_charset or DSN (in case of PDO) were used for the purpose - you are safe again.

As for PDO, a tag wiki should be enough for starter, I believe

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Thank you for your answer, but the code you provided makes the matters worse. First of all, the %'s used around the addCslashes is useless, as these can be placed around the $q while SELECTING (which I do). Lastly, stripslashes is needed for the output, or else it will return "jack\'s returned x results" instead of "jack's returned x results" All in all, the above code is nearly identical to mine -- although yours adds slashes on \'s which I'm going to steal! – AnonymousJ Apr 21 '13 at 19:48
  • I am sorry, but both your ideas are quite wrong. With %s added to variable you can use it either for escaping or binding. I **wouldn't** call it "worse". And stripslashes are **never** needed to read data from database. If you need it, that means the you have your data spoiled, either by magic quotes or whatever else wrong code. And you have to clean it up instead of using stripslashes. – Your Common Sense Apr 21 '13 at 19:55
  • I think you have misunderstood something here (and maybe I have not been clear in my post). My output is not coming from the database (as you indicate), but from what the user has typed as an input. So yes, it does matter where I add the %s, and it is important to use stripslashes, as I'm **not** reading the data from the database in the case of the output. – AnonymousJ Apr 21 '13 at 19:59
  • That's right. You need 2 variables for this. My apologies. Corrected the code. – Your Common Sense Apr 21 '13 at 20:09
  • No problemo. By the way, when I use `htmlspecialchars` on the output, it seems something like "Ömer" does not show up, because of the "ö". Could this be because of some contradicting charset settings? – AnonymousJ Apr 21 '13 at 20:31
  • it shouldn't an issue. are you sure it is shown without htmlspecialchars all right? – Your Common Sense Apr 21 '13 at 20:34
  • Yes I am. Currently I'm using ISO-8859-1, but in the near future I probably would switch to UTF-8. But I just created a new file, where I tried `htmlspecialchars` on "Ömer". While the file is encoded in UTF-8, I tried setting the meta charset to ISO-8859-1, which made it display nothing, while setting it to UTF-8 resulted in it displaying "Ömer". So I guess it has something to do with charsets. This is on my local host with PHP 5.4.6, while the live version of my script with PHP 5.3.19 displays it correctly. – AnonymousJ Apr 21 '13 at 20:39
  • Can I perhaps ask you a quick question? Would it be any secure risk, if I didn't escape `<` or `>` through the use of `htmlspecialchars`, `htmlentities` or `strip_tags` on the input? As the input already trims whitespace, escapes with `mysql_real_escape_string` and adds slashes to `\`, `%` & `_`, should I even think about using the aforementioned functions to escape possible HTML tags from the GET? Because whenever I use `htmlspecialchars`, it also escapes ampersand (&), which leads to no results coming up for something like `John & Eve` (guess the same goes for `htmlentities`). (Continued) – AnonymousJ Apr 22 '13 at 22:18
  • (Continued) And `strip_tags` makes it possible for a person to search with something like this `John & Eve`, which displays results for `John & Eve`, which I see a problem with. – AnonymousJ Apr 22 '13 at 22:19