3

It's a well covered topic, but I'd like to get some confirmation on methods of using data from user variables, in a few different situations.

  1. The variable is never used in a database, never stored, only displayed on screen for the user. Which function to use to make sure no html or javascript can screw things up?

  2. The variable is taken into the database, and used in SQL queries.

  3. The variable does both.

At the moment I xss_clean, and strip_tags. I've always done this, just by autopilot. Is there a better technique? Apologies if there's an identical question out there. I kinda assume there is, although I couldn't find one as thorough as this.

Cheers.

Michael Watson
  • 1,079
  • 3
  • 14
  • 26
  • possible duplicate of [The ultimate clean/secure function](http://stackoverflow.com/questions/4223980/the-ultimate-clean-secure-function) – Your Common Sense Mar 31 '12 at 13:20

4 Answers4

5
  1. Use the appropriate function while outputting, in HTML context, this is htmlspecialchars
  2. Use prepared statements
  3. See 1. and 2. – depending on whether you are displaying the variable or you are using it in a query.
knittl
  • 246,190
  • 53
  • 318
  • 364
  • 3
    There is currently a buffer overflow in PHP 5.4 htmlspecialchars,htmlentities. http://www.fortiguard.com/encyclopedia/vulnerability/php.htmlspecialchars.htmlentities.buffer.overflow.html – Lawrence Cherone Mar 31 '12 at 13:18
  • @LawrenceCherone What should be done to combat the vulnerability? – qitch Mar 31 '12 at 13:24
  • possible only allow maximum 40 bytes, so split the string. – Lawrence Cherone Mar 31 '12 at 13:25
  • 1
    And use `ENT_QUOTES` as the second parameter for [htmlspecialchars()](http://php.net/htmlspecialchars) if you output a string in an input _value_ attribute and if you use single quotes. – ComFreek Mar 31 '12 at 13:25
  • This knowledge on htmlspecialchars is great. Thanks. Can you elaborate further on "used prepared statements?" I'm primarily using mysql, so is mysql_real_escape_string good enough? – Michael Watson Mar 31 '12 at 13:57
  • @Michael it is not. To let you know, mysql_real_escape_string has nothing to do with security at all. – Your Common Sense Mar 31 '12 at 14:11
  • @YourCommonSense show me an injection that will work after escaping and running through `mysql_query`. – qitch Mar 31 '12 at 14:12
  • @qitch: make that »properly escaped« and I'm all with you. – knittl Mar 31 '12 at 14:15
  • @qitch you have seen it in the other answer comments. – Your Common Sense Mar 31 '12 at 14:16
  • @YourCommonSense Those do not work. Why? Because `mysql_query` ignores after the first statement. – qitch Mar 31 '12 at 14:18
  • @qitch you are getting too noisy. Read my lips: This is an example of the **SQL. CODE. INJECTION.** The code was injected. You have got what you were asking for. If you don't understand what sql injection is - please, learn it first. I'd be happy to continue our conversation after that. Have a nice day. – Your Common Sense Mar 31 '12 at 14:21
  • @YourCommonSense And I said "injection that will *work*." It means nothing if you inject characters that look like code if they aren't treated like code. – qitch Mar 31 '12 at 14:24
  • @Lawrence: that vulnerability only affects use of the `double_encode`-false argument, something that's only really needed to ‘clean up’ broken HTML. It's a bad, bad bug, but luckily it does not affect normal use of `htmlspecialchars` to do plain HTML escaping. – bobince Mar 31 '12 at 15:34
4

One of worst delusions in the PHP world is that $_GET or $_POST have anything to do with security.

It is not the source but destination that matters

  • If you have to deal with database, the rules always the same, no matter if the data is coming from $_POST, SOAP request or a database. It has to be ALWAYS the same: placeholders for the data, whitelisting for the everything else.
  • If you have to output some data into browser, you have to properly prepare it, no matter whether the data is coming from $_POST, SOAP request or a database.
  • If you have to read from a file - you have to secure the filename, no matter where it coming from, and so on
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 1
    -1 for being unhelpful. You might as well had just said "Make your data safe." – qitch Mar 31 '12 at 13:41
  • Erm, yes. I clearly ask for a solution to both input types, implying that there is no differentiation. – Michael Watson Mar 31 '12 at 13:51
  • 2
    @Michael please read my answer *attentively*. You don't need no special security for the POST.You need it for the database. Don't you understand that? – Your Common Sense Mar 31 '12 at 14:08
  • 3
    Thanks for your answer. You were a deadly combination of helpful and angry, like a college professor in the midst of a divorce. I do understand that the source is irrelevant, and sorry if I didn't make that clear. Your other points throughout this question are good, and I've cleared a lot up in my mind. I'm largely using CI & active record classes, so this is largely educational for me, just make sure I totally understand what's happening in my applications. Thanks again. – Michael Watson Mar 31 '12 at 23:05
-1
  1. In the first case htmlspecialchars() probably is the best choice, allowing for users to use all characters like <, >, &, etc.
  2. In the second case you will need to use some database escaping function like mysql_real_escape_string or a prepared statement with PDO or mysqli. Prepared statements are the best choice here but if you are only familiar with mysql then mysql_real_escape_string works fine too. If you are not using mysql then there are similar functions in most SQL APIs.
  3. In the third case do both but separately, with gives you two diffrent results, one for output and one for database.

References:

http://php.net/manual/en/function.htmlspecialchars.php

http://php.net/manual/en/function.mysql-real-escape-string.php

http://php.net/manual/en/book.pdo.php

http://php.net/manual/en/book.mysqli.php

Andreas Hagen
  • 2,335
  • 2
  • 19
  • 34
  • escaping has nothing to do with security. mysql_real_escape_string IS NOT works fine. -1. Shame to one who upvoted. – Your Common Sense Mar 31 '12 at 13:28
  • Well yes it does, if you do not properly escape data sent to databases like MySQL and such you will leave it vulnerable to SQL injections. This is also true for outputted data in HTML which will be vulnerable to XSS attacks if you do not escape/encode the output properly. – Andreas Hagen Mar 31 '12 at 13:31
  • `$id="1;drop table users;"; $id=mysql_real_escape_string($id); $sql="SELECT * FROM table WHERE id=$id";` eat it – Your Common Sense Mar 31 '12 at 13:34
  • I get your point, but you made the mistake of escaping something which was not sent as a string. It would however work if you sent it as a string. It is a crude way to do it and one should of course always validate integers with is_numeric and intval but if you instead used mysql_real_escape_string and wrapped your id in quotation marks, thus sending it as a string, MySQL would ignore the extra data in the id (;drop table users;) and just fetch you the row with id 1. `SELECT * FROM table WHERE id="1;drop table users;"` – Andreas Hagen Mar 31 '12 at 13:41
  • **It is not my mistake but yours**. There is nothing in your answer about strings. And even if there was, change the query to `$sql="SELECT * FROM table LIMIT $id";` and try to excuse yourself again. – Your Common Sense Mar 31 '12 at 13:44
  • I should maybe have made it a little more clear that my answer was not intended to be read by total morons. My mistake. I clearly said though that the best way to do it was with prepared statements, which would have worked, and just supplied with mysql_real_escape_string just to have mentioned it. Your example here expects a integer and thus would not work, I agree, and integer validation is necessary. If you have something more to discuss i suggest we take it in the chat or something instead of polluting this thread. – Andreas Hagen Mar 31 '12 at 13:51
  • I'd prefer to see your wrong answer plainly deleted rather than explaining the same topic again and again to another arrogant newcomer. – Your Common Sense Mar 31 '12 at 14:07
  • @qitch In that case I think you are doing something wrong, or maybe our MySQL servers differs somehow, anyways, my query was just intended to point out YourCommonSense's mistake, do not use it like that, validate instead :) (is_numeric and intval) – Andreas Hagen Mar 31 '12 at 14:10
  • Well, I was forced to assume that you called the OP a moron. Because after reading your answer he is asking if escaping is enough ;) But I have another version - it is not the OP, but your answer is wrong. – Your Common Sense Mar 31 '12 at 14:13
  • @AndreasHagen Sorry, I think you misunderstood. I was talking about the supposed sql injections. – qitch Mar 31 '12 at 14:13
  • Oh my mistake then, and the SQL injections is just a side effect you might encounter if you do it wrong, it isn't always the result :) For example if you do not escape a string at all and use single quotation marks for the string in the query then input like: ' UNION SELECT ... ' could be misused. – Andreas Hagen Mar 31 '12 at 14:19
  • @YourCommonSense Just to clarify: I called you a moron, not OP. – Andreas Hagen Mar 31 '12 at 14:47
  • Too bad, it missed the goal and hit the innocent person :) Seriously, don't blame someone for your own fault. It want make you many friends. Cheers :) – Your Common Sense Mar 31 '12 at 14:53
-3
$id="1;drop table users;"; $id=mysql_real_escape_string($id); $sql="SELECT * FROM table

WHERE id=$id";
jmr333
  • 183
  • 1
  • 9