character encoding and site security

Question

function intfix($i)
{
   $i = preg_replace('/[^\d]/', '', $i);
   if (!strlen($i))
      $i = 0;
   return $i;
}


function textfix($text = ""){
if(!is_array($text)){ $text = htmlentities($text,ENT_QUOTES,"UTF-8");
}
return $text;
}

These two functions filter all the user submitted variables. Do you think it is secure enough?

I'm a little confused about character encoding. I want to allow my users to play around with ACII art and use any kind of symbols they want but at the moment it doesn't seem to be possible. What should be done? It may have something to do with the table's encoding as well as my functions.

EDIT:

The numbers actually are really big. Sometimes in trillions.

This is an example how I filter user input:

if($_GET['number']){ $number = intfix($_GET['number']);  }
if($_GET['text']){ $text = textfix($_GET['text']);  }'

Is that the mistake you are talking about?

Also, this is how I filter them before inserting to the db:

function filter($input,$s=1){

    $input = strip_tags($input, "");
    $input = str_replace("\n", "<br />", $input);
    if($s == 1){$input = bbcode($input); } // smileys and bbcode
    $input = textWrap($input); // wordwrap without breaking html
    return $input;
}

function unfilter($input){ // to unfilter in case I need to show the text in a textbox

    $input = html_entity_decode($input,ENT_QUOTES,"UTF-8");
    $input = str_replace("<br />", "\n", $input);

    return $input;

}

score 1 · Accepted Answer · answered Oct 17 '11 at 08:29

1

Replace intfix with intval() or floatval() - you are reinventing the wheel unless you are expecting very large numbers.

I hope you are not using textfix() on input?? That would be a very big mistake. You must encode entities ONLY on output, not on input.

For UTF-8 you probably need:

ini_set('default_charset', 'UTF-8');

answered Oct 17 '11 at 08:29

Ariel

25,995
5
59
69

The numbers actually are really big. Sometimes in trillions since it's a mmorpg game. This is an example how I filter user input: if($_GET['number']){ $number = intfix($_GET['number']); } if($_GET['text']){ $text = textfix($_GET['text']); }' Is that the mistake you are talking about? – domino Oct 17 '11 at 17:28
I added two other functions to my main post showing how I filter the data before inserting to the db. Hope you can take a look. – domino Oct 17 '11 at 17:36
No, no, no. This is no good. Your intfix is OK if you really do have numbers larger than will fit in a 32 bit signed int. (Although I'm surprised you have numbers that large.) But the rest is no good at all. The textfix and filter should be done on OUTPUT *NOT* on input! For the database use `mysql_real_escape_string()`, and nothing else at all. When you show it on the page, then you can filter etc. – Ariel Oct 18 '11 at 02:21
So I shouldn't filter input at all? Why does it make such a difference when I do it? I guess I'm going to have to re-code the majority of my site. – domino Oct 18 '11 at 03:00
You should not filter on input because html is not the only output. You can store in a database, and perhaps search it, you might email it. Also and by far the most important: The filtering depends on what you do with the output! Filtering data for an `alert()`, a `style` attribute, a persons name in html, a persons name in ajax then used in javascript, a name in the title tag - every one of those needs different filtering. It is impossible to make a general purpose filter, if it was possible this stuff would be a lot easier, but filtering (escaping really) needs to be specific to the context. – Ariel Oct 18 '11 at 05:24
Feel free to check and comment on my revised version: [link](http://stackoverflow.com/questions/7946652/is-this-enough-for-a-secure-site-4-small-functions/) – domino Oct 30 '11 at 17:55

score 0 · Answer 2 · answered Oct 17 '11 at 08:28

0

The intfix() function is not necessary, you can simply typecast to int by doing $num = (int)$num;

As for string sanitization, if you cannot use parametrized queries, pass your string through myqsl_real_escape_string().

answered Oct 17 '11 at 08:28

code_burgar

12,025
4
35
53

mysql_real_escape_string has absolutely no value when you're displaying something. alert( 'hello' ) will still be alert( 'hello' ), keeping your website open to XSS. – Berry Langerak Oct 17 '11 at 08:36
Since the question is tagged mysql and OP includes a reference to a number sanitization function, mysql_real_escape_string is most definitely valuable. Storing user input in a DB without sanitizing possible SQL injections is a far bigger problem than not screening input for XSS attempts. XSS can be dealt with on output, as Ariel said. – code_burgar Oct 17 '11 at 09:38
Obviously mysql_real_escape_string is the best tool for protection, but I'm just saying that blindly applying it to everything thinking you're safe, doesn't solve all of your problems. – Berry Langerak Oct 17 '11 at 10:29

character encoding and site security

2 Answers2