MrMoo posted:

It looks like mb_check_encoding() would be a good choice,

http://hk2.php.net/manual/en/function.mb-check-encoding.php

Pookster posted:

I just stick this near the beginning of my script (I route everything on a site through one script) and then get on with things as normal.

php:

<?
// Kill magic quotes because it's stupid.
if (get_magic_quotes_gpc()) {
    function stripslashes_deep($value) {
        if (is_array($value)) {
            $value = array_map('stripslashes_deep', $value);
        }
        else {
            $value = stripslashes($value);
        }
        return $value;
    }
    $_POST = array_map('stripslashes_deep', $_POST);
    $_GET = array_map('stripslashes_deep', $_GET);
    $_COOKIE = array_map('stripslashes_deep', $_COOKIE);
    $_REQUEST = array_map('stripslashes_deep', $_REQUEST);
}
?>

DarkLotus posted:

FYI, you can use $_POST = stripslashes_deep($_POST); since your function already does an array_map.

Hammerite posted:

For information, and in relation to the discussion we were having about sanitising user input, this is the function I have adopted for sanitising user input that is expected to be of type string. (PHP_MAJOR_VERSION is defined elsewhere.)

php:

<?

define( 'STRING_TO_UPPERCASE'   ,  1 );
define( 'STRING_TO_LOWERCASE'   ,  2 );
define( 'STRING_NO_TRIM'        ,  4 );
define( 'STRING_NO_STRIP_CR'    ,  8 );
define( 'STRING_STRIP_NEWLINES' , 16 );
define( 'STRING_ESCAPE_HTML'    , 32 );
function sanitise_string ($x, $flags = 0) {
    $x = (string)$x;
    if ( PHP_MAJOR_VERSION < 6 and get_magic_quotes_gpc() ) {
        $x = stripslashes($x);
    }
    if ( $flags & STRING_TO_UPPERCASE ) {
        $x = strtoupper($x);
    } else if ( $flags & STRING_TO_LOWERCASE ) {
        $x = strtolower($x);
    }
    if ( ( $flags & STRING_NO_TRIM ) == 0 ) {
        $x = trim($x);
    }
    if ( ( $flags & STRING_NO_STRIP_CR ) == 0 ) {
        $x = str_replace("\r", '', $x);
    }
    if ( $flags & STRING_STRIP_NEWLINES ) {
        $x = str_replace("\n", '', $x);
    }
    if ( $flags & STRING_ESCAPE_HTML ) {
        $x = htmlspecialchars($x);
    }
    return $x;
}

?>

isagoon posted:

No offense, but this is a pretty good example of how not to write code. So much of this should be reformatted or otherwise changed.

Hammerite posted:

I'd love to respond to this comment, but you seem to have gone out of your way to provide absolutely no specific criticisms whatsoever.

gwar3k1 posted:

Your ifs use inconsistent notation: "and" "&". Shouldn't ands be && or is & something specific.

quote:

$flags & STRING_TO_UPPERCASE will always equal true

gwar3k1 posted:

Why would you remove new lines and not replace them with some identifier so that the formatting matches when the string is drawn from the database? I know this pisses off users just from seeing people's Facebook status complaining about the fact line breaks used to be removed.

isagoon posted:

No offense, but this is a pretty good example of how not to write code. So much of this should be reformatted or otherwise changed.

Hammerite posted:

You know, I looked back at that function and decided I didn't like the way it was laid out very much. I have no idea what your objections are to it, but I reformatted it to be more pleasing. Or at least, more pleasing to me. I have no idea where you are coming from.

code:

define( 'STR_TO_UPPERCASE'   ,  1 );
define( 'STR_TO_LOWERCASE'   ,  2 );
define( 'STR_NO_TRIM'        ,  4 );
define( 'STR_NO_STRIP_CR'    ,  8 );
define( 'STR_STRIP_NEWLINES' , 16 );
define( 'STR_ESCAPE_HTML'    , 32 );
function sanitise_str ($x, $flags = 0) {
    $x = (string)$x;
    if ( PHP_MAJOR_VERSION < 6 and get_magic_quotes_gpc() ) {
        $x = stripslashes($x);
    }
    if (  $flags & STR_TO_UPPERCASE   ) { $x = strtoupper($x);            }
    if (  $flags & STR_TO_LOWERCASE   ) { $x = strtolower($x);            }
    if ( ~$flags & STR_NO_TRIM        ) { $x = trim($x);                  }
    if ( ~$flags & STR_NO_STRIP_CR    ) { $x = str_replace("\r", '', $x); }
    if (  $flags & STR_STRIP_NEWLINES ) { $x = str_replace("\n", '', $x); }
    if (  $flags & STR_ESCAPE_HTML    ) { $x = htmlspecialchars($x);      }
    return $x;
}

isagoon posted:

Ok, allow me to explain.

First off, $x = (string)$x; is unneeded. If the developer supplies an object or something that != a string to your function, it isn't the functions fault and casting the argument to a string isn't going to accomplish much (unless, that is you are passing it a class with a tostring magic method). Even if you did do something like that, I believe that PHP will automatically case the object to a string. Any way you look at it, that line does nothing.

quote:

Second, single sentence blocks should not be braced.

quote:

Third, but nitpick, is that you may want to use && instead of AND. Either will work, but most everybody prefers to use &&.

quote:

Since you aren't doing anything with $x after your if statement, you could just return each one of those right then and there instead of reassigning it.

Hammerite posted:

You might well be right, but making sure it's a string when it comes out the other end of the function gives me peace of mind.

You aren't making sure of anything. PHP will cast it to a string no matter what it starts out as, and those function calls will return a string no matter what. If something other than a string goes into your function, something severely hosed is going on. Also, if what you pass is not a string, that sentence is not going to do anything to prevent something bad happening later on.

I disagree. Every block must be braced.

If you say so, I guess. I prefer "and". Providing "and" and "or" as keywords is one of the vanishingly few things PHP can claim to have as an advantage over other languages (trivial though it is).

and that's exactly why you shouldn't use them

This is not true. I might be (probably will be) doing more than one thing to $x, depending on the value of $flags.

As you wish. For that code snippit, you could just return immediately

i am not zach posted:

Having an issue with an array and a foreach loop, trying to figure out whats going on...

My question is how on earth is $arrOptionData['default'] set to '0' and 'MacBook (Unibody)' instead of 'default' and 'MacBook'

edit. And it only happens if the very first value in $arrFieldData['options'] is the default... otherwise works as I intended

isagoon posted:

You aren't making sure of anything. PHP will cast it to a string no matter what it starts out as, and those function calls will return a string no matter what.

isagoon posted:

As you wish. For that code snippit, you could just return immediately

Hammerite posted:

Try changing if ($strOptionValue == 'default') to if ($strOptionValue === 'default'). I think you're unintentionally getting a result of "true" from the comparison when PHP tries to compare a non-numeric string to integer 0.

isagoon posted:

As you wish. For that code snippit, you could just return immediately

isagoon posted:

Second, single sentence blocks should not be braced.

Hammerite posted:

Try changing if ($strOptionValue == 'default') to if ($strOptionValue === 'default'). I think you're unintentionally getting a result of "true" from the comparison when PHP tries to compare a non-numeric string to integer 0.

What if I use

$x = sanitise_str(@$_GET['something_or_other'], STR_NO_TRIM | STR_NO_STRIP_CR);

and magic_quotes_gpc is off? Then it'll pass through the function unchanged. And the input might indeed happen not to be a string if I'm using it on GET or POST data.

No, just no. No matter what GET or POST data you are processing, it will either be a string or any array. Manually casting the input to a string whether it's an array or a string will do _nothing_. A string can't be "sort of" or "kind of" a string, it's a string. You cannot send objects through GET or POST, but you can send representations of objects, as well as representations of arrays. If you do such a thing with an array, PHP will automatically convert it to an array.

I don't understand what you mean. The only one of those "if" statements where I could replace the assignment with a "return" statement and not change the function's behaviour is the last one, the htmlspecialchars() one.


Er, oh, I see. I didn't see that those were a bunch of different ifs pushed together.

isagoon posted:

barbarianbob posted:

If I was about to use a function named sanitise_str(), I would expect to be able to pass any user input to it and get a string as output. If you don't include the typecast, then the return value could be an array. If I'm expecting a string and I get an array, this could very well lead to an exploit.
And why would anyone spend the time to decide whether a typecast is superfluous? Even if it is, you might make a change to the code later to something that would require that typecast.

isagoon posted:

I really don't understand this. It isn't permissible in any other language, why should you expect valid output when you input something totally unexpected?

Hammerite posted:

Well, the thing is, if the function is being used to tackle user-supplied input, you're opening it up to input that is totally unexpected. Given that the input is received from a user who's using your site faithfully, you have nothing to worry about. If you're receiving input from a malicious user, it's as well to make sure your public-facing scripts coerce the input to something harmless and do their job as well as possible. It's not as though doing a simple typecast adds a lot of work for the server to do.

isagoon posted:

No, it doesn't. The user cannot supply anything but a string. The only way a user could supply an array is if he used a script to POST (or GET, for that matter) an array of strings. Remember, there is a difference between a string representation of an object (RE: Serialization) and an actual data type.

ShoulderDaemon posted:

If a user sends the request script.php?foo=a&foo=b then $_GET["foo"] will be an honest-to-god array. This is trivial for a user to do, and doesn't require any script or programming or anything on the user's end. If the script is expecting $_GET["foo"] to be a string, then bad things could happen because various operations will not perform as the programmer expected. Explicitly casting it to a string removes this possibility.

FeloniousDrunk posted:

What, I always had to do script.php?foo[]=a&foo[]=b to get $_GET['foo']=array('a','b')... did things change? I know I've seen Perl work like the above...

ShoulderDaemon posted:

It's possible I'm misremembering and you're correct; it's been more than a while since I had to use PHP.

FeloniousDrunk posted:

What, I always had to do script.php?foo[]=a&foo[]=b to get $_GET['foo']=array('a','b')... did things change? I know I've seen Perl work like the above...

isagoon posted:

You're right, that should produce an array, which is what I have been saying.

A better alternative to simply casting would be to check if it's an array and return false or throw an exception.

Hammerite posted:

No you haven't! You've been insisting all along that the typecast is redundant and will make no difference, and that "the user cannot supply anything but a string" (your words). The best sense I can make of your posts is that you're making some academic point about how if the user sends an array then it will be serialised in the form of a string, which doesn't have any bearing on the fact that it'll be an array by the time my script sees it. Now you're recognising that it can be an array, but saying "err, yeah it could be an array, but you should do something different to deal with the possibility anyway"? As if it matters which of various options I use to deal with (presumably) malicious input, provided that it's made safe.

isagoon posted:

Ok, we're talking about two different things there.

First off, if you input an array into a function that wants a string, you will get "Array". There is no way to find an exploit off this. An array tostring is just "Array".

isagoon posted:

Second, if you want to catch that condition, you shouldn't just blindly cast it to a string. You will get the same as above. Try it for yourself if you don't believe me. If you want to catch it, you should be specifically checking if it's an array.

Regardless of if you manually cast it to a string, you will still get "Array".

quote:

I know that if you cast an array to string, you get "Array". I'm not sure why this matters

Hammerite posted:

Should I be checking user input to make sure it is well-formed UTF-8? Is there a builtin function for that? What is the worst-case scenario if I fail to screen for bad UTF-8?

quote:

Typecasting to string argument

Trinitrotoluene posted:

I know %22 is ASCII for " but why the hell is it invading my url and malforming it!!

Hammerite posted:

Unfortunately that means that e.g.

code:

[url="http://www.google.co.uk]Google[/url]

(with only a beginning quote or only an ending quote) will be regarded as a valid BBCode URL tag. I am not good enough at regular expressions to help further.