Security Question

[Nickson]

Well attitude matters... I'm not surprised that some members do not wish to share their knowledge. We should be thankful to those that are sharing their knowledge and experience freely. They don't have to do it if they don't want too, and I'm quite sure they can use the time for something else as well if they feel unwanted. I would love to have a few more topics like this!A topic with examples, tests, clean code, and an actual discussion. Something is really discussed in here, albeit with the typical arrogance, but it's still better than normal... and yet some want to ruin it for all.

[bluegman991]

It looks like to me a lot of this arguing is going on because of misunderstandings.

 

What if I put in a negative number? It fails right there.

Danny 1 - 0 Runthis

I copy/pasted runthis code and tested it. He had 1 syntax error which was a missing semicolon.

On the php page I echo'd sanitizeOne($post,'int');

I created an html page with a form and input.

input: 1

output: 1 (success: 1 is valid integer)

input: -1.5

output: -1 (fail: -1.5 is not a valid integer. Integer value of -1.5 is -1. Script success!)

input: 1.5

output: 1 (fail: 1.5 is not a valid integer. Integer value of 1.5 is 1. Script success!)

input: -1

output: -1 (success: -1 is a valid integer)

Will you please elaborate on how, where, or why this script failed when -1 is indeed an integer.

______________________________________________________________________________________________________________________________________________________

runthis asked: can you explain the situation in which you are asking the user to enter in a negative number?

Were not, thats the point. Ive seen people gain millions on games from putting in negative numbers.

Seems like its 2-0 to me :)

The integer securing part of this script was to secure against non integer input. In some cases negative numbers maybe wanted (For example an answer to a math problem 2-5).

If someone wanted to stop negative numbers from being input they could do a simple range check or simply wrap sanitizeOne with the abs function. Just because a script allows negative numbers doesn't mean its bad.

________________________________________________________________________________________________________________________________________________

 

If you are using your function for all variables, any item where you can input a number will be a problem, like adding an amount of crystals to a market, or playing a 50/50 game for example. As the one function is used all over the site, every place will be at risk, rather than just one.

As I said to danny the script is for validating if a user inputs a valid integer. Floating numbers are converted to integers and strings are converted to 0.

You guys are only considering 1 reason a user would be entering a number (To add or subtract it from another number to get a positive number).

If you don't desire negative numbers just do a range check or get the absolute value of the input number. Just know that sometimes negative and positive numbers are desired and.

Think about this: someones captcha system was to answer a math question 2-5. If they ran the input through a function that only allowed positive numbers. Then checked if 2-5 == $input. Then the user input answer would always be wrong.

_________________________________________________________________________________________________________________________________________________________

[bineye]

As I said to danny the script is for validating if a user inputs a valid integer. Floating numbers are converted to integers and strings are converted to 0.

You guys are only considering 1 reason a user would be entering a number (To add or subtract it from another number to get a positive number).

If you don't desire negative numbers just do a range check or get the absolute value of the input number. Just know that sometimes negative and positive numbers are desired and.

Think about this: someones captcha system was to answer a math question 2-5. If they ran the input through a function that only allowed positive numbers. Then checked if 2-5 == $input. Then the user input answer would always be wrong.

It appears you misunderstood what I was saying. I was backing up the point that the negative integers were not filtered, and it was said:

this is a fast and easy way to never have to worry about xss,sql injections

I took this to mean he put this in a globally accessible file and used it on every variable. Therefore, as games require a number of instances where a user has to put in an integer, then this would be open to exploitation on pages like the ones I gave as examples. And then as you gave an instance where a negative integer would be required, then this renders this portion of the function useless, as in some case, the function will not work as desired.

[BeastTheKidd]

Question

Can you not first check if it is a valid integer, and then search and destroy any unwanted signs in front of or after the digits?

[runthis]

Thank you bluegman991 for doing tests against the post i made instead of just assuming it fails. I really felt like i was being attacked for sharing my method. I always thought i was using classic php methods that are able to suit most peoples needs. I guess i expected Danny to exclaim how the code is bad without providing his own examples. Shame really. Thanks again bluegman991, i must say that the script is not for everyone, as my original post says, you need to modify it to fit your needs as i did mine a year or two back.

To the above reply, spool through this thread for some good code from all of us, a few people mentioned using the ctype function, you can also multiply any expected positive number variable by 1 to confirm if its a digit, you can preg_match it as well.

If you put all php aside and just think logically mathematically what you could do to confirm its a number, add one digit and than subtract a digit, this automatically removes any character input after any integers, if it starts with a character input instead of integer, it returns 0, unless the number 0 is a real common thing in your script, you could deny if its zero. It can depend on the situation alot of times. To really really answer your question, there is no inherit php function that automatically cleans all your variables, although i sure wish there was.

Edited December 30, 2011 by runthis

[lucky3809]

everyone secures differently then others, I have seen numerous secure features. The best thing to do is stick to what you know works, that you understand!

[Danny696]

Thank you bluegman991 for doing tests against the post i made instead of just assuming it fails. I really felt like i was being attacked for sharing my method. I always thought i was using classic php methods that are able to suit most peoples needs. I guess i expected Danny to exclaim how the code is bad without providing his own examples. Shame really. Thanks again bluegman991, i must say that the script is not for everyone, as my original post says, you need to modify it to fit your needs as i did mine a year or two back.

To the above reply, spool through this thread for some good code from all of us, a few people mentioned using the ctype function, you can also multiply any expected positive number variable by 1 to confirm if its a digit, you can preg_match it as well.

If you put all php aside and just think logically mathematically what you could do to confirm its a number, add one digit and than subtract a digit, this automatically removes any character input after any integers, if it starts with a character input instead of integer, it returns 0, unless the number 0 is a real common thing in your script, you could deny if its zero. It can depend on the situation alot of times. To really really answer your question, there is no inherit php function that automatically cleans all your variables, although i sure wish there was.

Sorry if you felt I was attacking you, I was making the point, I did say it very badly though, and I shouldn't have, for which I do apologize, but you did say the function would secure the game, and as bluegman even said, it doesnt filter out negative numbers, which could cause a user to gain money, which could ruin a game. Making it not as secure as you lead people to believe.

[BeastTheKidd]

Going back to the OP's question and assuming a simple GET, rather than

the much more complex POST, there are a limited set of cases to look at:

Integers, where the incoming "var" will take on the form of a positive

integer; usually below 2^31:

-- http://www.domain.com/page.php?var=123

-- http://www.domain.com/page.php?var=0

-- http://www.domain.com/page.php?var=9999999

Options, where the incoming "var" will be a short string that matches

one of a small set of predefined strings:

-- http://www.domain.com/page.php?var=red

-- http://www.domain.com/page.php?var=GREEN

-- http://www.domain.com/page.php?var=Blue

Strings, where the incoming "var" cannot be known in advance, but may

need further validation such as usernames or email addresses.

-- http://www.domain.com/page.php?var=Octarine

-- http://www.domain.com/[email protected]

In all three cases, the first thing you ideally need to is determine

where the information came from. Sounds daft, but a POST request also

populates the _GET superglobal, so:

 

if ($_SERVER['REQUEST_METHOD'] == 'GET')
{
   /**
     * Continue our checks here...
     **/
}
else
{
   /**
     * In some cases, it is sufficient to return a 400 (Bad Request)
     * here, but that will be highly application dependant.
     **/
}

 

You need to assert that the variable you were looking for was actually

posted, and while isset() is a common method, it does have drawbacks, so

the array_key_exists() is the function of choice these days:

 

if (array_key_exists('var', $_GET))
{
   /**
     * Continue our checks here...
     **/
}
else
{
   /**
     * If we wer expecting the var to be populated, we could display
     * an error message here, redirect or issue a 400.
     **/
}

 

Next, type checking. Both the _GET and the _POST superglobals can hold

strings or arrays, even though HTTP is at heart a text-based protocol.

This is a mangling that PHP is doing for you; it can in fact be useful

when dealing with certain complex POST requests, particularly radio

buttons and checkboxes.

For now, you will almost always be checking for a string type:

 

if (is_string($_GET['var']))
{
   /**
     * Continue our checks here...
     **/
}
else
{
   /**
     * Again, up to you. Error message, redirect or 400.
     **/
}

 

Having got this far, we are assured that there is a string value in

$_GET['var'], however we still need to examine it depending on what type

of data is supposed to contain.

For integers, assuming you are looking at positive integers, then really

nothing more fancy than ctype_digit() is needed.

 

if (ctype_digit($_GET['var']))
{
   /**
     * Continue our checks here...
     **/
}
else
{
   /**
     * Nasty, there is an odd character (not a decimal digit) in the
     * var. You could use a default value, but really, it should be
     * an error message or a 400.
     **/
}

 

Sticking with integers, you probably want it actually in integer form

rather than in string form, and this usually catches out a large number

of people.

Assuming the number is of a sane size; ie less that 2^31, then intval()

will work fine. Above that, then you will see differences between 32 and

64-bit boxes and numbers above 2^31/2^63 will be clamped.

ie: on my 64-bit test box:

 

$ php -r "echo intval('12345678901234567890');"
9223372036854775807

 

Of course then have to perform bounds checking on the value. Does it

fall within two acceptable values, or does it match a particular value?

For string options, having already asserted the value is a string, you

need to see if it matches one of your pre-defined options:

 

$options = array('Red', 'Green', 'Blue');

if (in_array($_GET['var'], $options))
{
   /**
     * We have a match!
     **/
}
else
{
   /**
     * The var was not one of Red, Green or Blue. But... and here's the
     * rub, in_array() is case sensitive, so really, this is a poor
     * check.
     **/
}

if (in_array(strtolower($_GET['var']), array_map('strtolower', $options)))
{
   /**
     * We have a match!
     * This is a much better method as it allows in this case:
     * RED, green, BluE to match Red, Green and Blue respectively.
     **/
}
else
{
   /**
     * No match
     **/
}

 

For the more complex strings such as username or email address, then

by all means use a regular expression:

 

$regex = "^...$";

if (preg_match('`' . $regex . '`i', $_GET['var']))
{
   /**
     * We have a match
     **/
}
else
{
   /**
     * Darn it. no match
     **/
}

 

Why keep the regex in a separate variable? Because that way it can be

seen by anybody with ease, plusi it is quite likely you can use it in

your javascript validation routines as well, so declaring it would allow

you to pass it to a template for example.

Putting all this together is remarkably easy; for simple integers:

 

$value = false;

if ($_SERVER['REQUEST_METHOD'] != 'GET')
{
   if (array_key_exists('var', $_GET))
   {
       if (is_string($_GET['var']))
       {

           // Begin data validation

           if (ctype_digit($_GET['var']))
           {
               $test = intval($_GET['var']);

               /**
                 * Now check to see if value matches something,
                 * or if it falls between an upper and a lower.
                 * (The lower bound can be ignored if 0).
                 **/

               if (($test >= 100) && ($test <= 999))
               {
                   $value = $test;
               }
               else
               {
                   die('Bounds error; "var" must between 100 and 999 inclusive');
               }
           }
           else
           {
               die('Syntax error; "var" must be a string of decimal digits');
           }

           // End data validation

       }
       else
       {
           die('Syntax error; invalid "var" type');
       }
   }
   else
   {
       die('Bad request; missing "var" parameter');
   }
}
else
{
   die('Request not supported');
}

/**
 * At this stage, $value will be false (assuming you remove or replace
 * the die() statements, meaning that the variable was not passed or
 * does not match your criteria.
 *
 * or...
 *
 * $value will contain an integer within your range of values.
 **/

 

For option strings:

 

$value = false;
$options = array('Red', 'Green', 'Blue');

if ($_SERVER['REQUEST_METHOD'] != 'GET')
{
   if (array_key_exists('var', $_GET))
   {
       if (is_string($_GET['var']))
       {

           // Begin data validation

           if (in_array(strtolower($_GET['var']),
                        array_map('strtolower', $options)))
           {
               /**
                 * Match!
                 **/

               $value = $_GET['var']
           }
           else
           {
               die('"var" must be one of ' . implode(', ', $options));
           }

           // End data validation

       }
       else
       {
           die('Syntax error; invalid "var" type');
       }
   }
   else
   {
       die('Bad request; missing "var" parameter');
   }
}
else
{
   die('Request not supported');
}

 

And for the more complex strings that require regular expression

matching:

 

$value = false;
$regex = '^...$';

if ($_SERVER['REQUEST_METHOD'] != 'GET')
{
   if (array_key_exists('var', $_GET))
   {
       if (is_string($_GET['var']))
       {

           // Begin data validation

           if (preg_match('`' . $regex . '`i', $_GET['var']))
           {
               /**
                 * Okay!
                 **/
           }
           else
           {
               die('"var" failed the regex check');
           }

           // End data validation

       }
       else
       {
           die('Syntax error; invalid "var" type');
       }
   }
   else
   {
       die('Bad request; missing "var" parameter');
   }
}
else
{
   die('Request not supported');
}

 

Phew!

Yes, it's long winded, and yes, it *is* convenient to drop a lot of

these tests into a function or set of functions however there is NO

security here. All I've done is validate the incoming data.

For integers, you will know whether they are within a range, or match

a particular value depending on your criteria; for string options, you

will know that your option is a valid one, and for free-form strings,

you will at least know they have been matched against a regular

expression.

Free-form strings if used to access filenames would need a few more

checks, and certainly storing them in a database ill require that they

be escaped in all but a few cases. Displaying them will require some

limited form of escaping, but that's a different subject.

POST'ed data is a little bit more complex to validate, with a number of

problems to overcome, however even sticking to this type of validate

whether it be GET or POST you will quickly have a much more stable site

than some.

[EDIT]

For free-form strings it should be noted that a length check is always

wise, as is compressing of whitespace and stripping nul characters

unless you have good reason not to.

I'd just like to ask a quick question, if its not too much to ask. Would array_change_key_case on the GET array for a certain script be sufficent, or is it better to use strtolower? Thanks, - Dillion