Using preg_match in my script

[ironzero]

Below is a script that passes data to a from with a URL, using for example: http://www,site,com/index.php?price=500&title=computer

[code]
<?php
function getIt($value){
  if($_GET[$value]){
    return $_GET[$value];
  }else{
    return "";
  }
}?>

<input type="text" name="price" value="<?php echo getIt("price")?>">
<input type="text" name="title" value="<?php echo getIt("title")?>">
[/code]

I'm still learning PHP, so my question is this...

I want to validate my input using preg_match, (I'm assuming this would be the easiest and most secure method).

How can I modify my script so that preg_match:

[b]ONLY[/b] accepts numbers, dollar signs and periods from:

<input type="text" name="price" value="<?php echo getIt("price")?>">

and [b]ONLY[/b] accepts values containing letters and numbers from:

<input type="text" name="title" value="<?php echo getIt("title")?>">


I want to use somthing like this:

if (preg_match('/^\d{5}(-\d{4})?$/',$_GET['zip'])) {
$zip = $_GET['zip'];
} else {
die('Invalid ZIP Code format');
}


Thanks!

[.josh]

[code]
<?php
  $num = '$123.45';
  // ^ is start of string $num
  // then match everything inside the [ ]
  // we only allow the following: dollar sign, numbers 0 through 9, and periods
  // * is for any amount of those allowed variables
  // $ is for end of string $num. the ^ at the beginning and $ on the end
  // makes sure that we only we only allow [$0-9.] for the entire string from
  // start to finish, not just some partial match inside the string
  if (!preg_match('/^[$0-9.]*$/',$num)) {
      echo "invalid";
  } else {
      echo "valid";
  }
?>
[/code]

[ironzero]

[quote author=Crayon Violent link=topic=119591.msg489962#msg489962 date=1166755226]
[code]
<?php
   $num = '$123.45';
   // ^ is start of string $num
   // then match everything inside the [ ]
   // we only allow the following: dollar sign, numbers 0 through 9, and periods
   // * is for any amount of those allowed variables
   // $ is for end of string $num. the ^ at the beginning and $ on the end
   // makes sure that we only we only allow [$0-9.] for the entire string from
   // start to finish, not just some partial match inside the string
   if (!preg_match('/^[$0-9.]*$/',$num)) {
      echo "invalid";
   } else {
      echo "valid";
   }
?>
[/code]

[/quote]

Ok, it's all starting to come together now...

but how would I tie this in with my getIt function?

Thank you very much for your help! ;)

[.josh]

well, i don't know exactly what it is you want to do here.  do you want to return some default? return an error message? Here is a starting point:

[code]
<?php
function getIt($value){
  if($_GET[$value]) { // value exists?
    if (!preg_match('/^[$0-9.]*$/',$_GET[$value])) { // is value NOT legal?
        // value failed the check. do something. assume returning nothing..
        return "";
    } else {
        // value passed the check.
        return $_GET[$value];
    } // end else passed check
  } else {
      // value doesn't exist. do something. assume returning nothing..
      return "";
  } // end else no value
} // end function getIt
?>

<input type="text" name="price" value="<?php echo getIt("price")?>">
<input type="text" name="title" value="<?php echo getIt("title")?>">
[/code]

[ironzero]

[quote author=Crayon Violent link=topic=119591.msg489979#msg489979 date=1166758253]
well, i don't know exactly what it is you want to do here.  do you want to return some default? return an error message? Here is a starting point:

[code]
<?php
function getIt($value){
  if($_GET[$value]) { // value exists?
     if (!preg_match('/^[$0-9.]*$/',$_GET[$value])) { // is value NOT legal?
        // value failed the check. do something. assume returning nothing..
        return "";
     } else {
        // value passed the check.
        return $_GET[$value];
     } // end else passed check
   } else {
      // value doesn't exist. do something. assume returning nothing..
      return "";
   } // end else no value
} // end function getIt
?>

<input type="text" name="price" value="<?php echo getIt("price")?>">
<input type="text" name="title" value="<?php echo getIt("title")?>">
[/code]

[/quote]


Thanks, that's exactly what I was trying to do...

although, how would I use separate preg_match statements for "price" and "title"?

Would I first have to define price and title as separate variables? I'm totally in the dark. :p

Thank you again!

[.josh]

according to your setup there, one of your input tags calls getIt with "price" as an argument, and the other one calls getIt with "title" as the argument.  Your function takes that "price" and "title" and checks to see if there is a $_GET['price'] and $_GET['title'] and then does the preg_match on it and returns it if it passes, or returns nothing if it fails.

Now, unless you want your 'title' to only contain numbers dots and dollar signs, you're gonna have to add a new preg_match to check for what you [i]do[/i] want to be allowed, and then throw a condition on top of that, to decide which preg_match you want to run, depending on what you pass to the function. 

[ironzero]

I see, so in other words it's easier to just use one preg_match that checks everything, right?

Also, I tried using:

die("Invalid value.");

instead of:

return "";


...but it doesn't seem to work, right. Instead of it displaying the message "Invalid value", it just shows "<input type="text" name="title" value="Invalid value."

basically I'm just trying to sanitize/secure everything so that the script doesn't receive any unexpected values.

Thanks! ;)


[quote author=Crayon Violent link=topic=119591.msg489994#msg489994 date=1166759966]
according to your setup there, one of your input tags calls getIt with "price" as an argument, and the other one calls getIt with "title" as the argument.  Your function takes that "price" and "title" and checks to see if there is a $_GET['price'] and $_GET['title'] and then does the preg_match on it and returns it if it passes, or returns nothing if it fails.

Now, unless you want your 'title' to only contain numbers dots and dollar signs, you're gonna have to add a new preg_match to check for what you [i]do[/i] want to be allowed, and then throw a condition on top of that, to decide which preg_match you want to run, depending on what you pass to the function. 
[/quote]

[.josh]

[quote author=ironzero link=topic=119591.msg489996#msg489996 date=1166760881]
I see, so in other words it's easier to just use one preg_match that checks everything, right?
[/quote]
well...not necessarily.. I mean, it depends on how picky you are.  For instance, that preg_match makes sure only dollar signs, numbers and dots are passed.  That probably isn't what you want for your title, right?  But if you include letters, then having a single preg_match would allow people to put letters into the price, as well.  So unless you can live with that, you're gonna want to build a custom expression for each field. 

[quote]
Also, I tried using:

die("Invalid value.");

instead of:

return "";


...but it doesn't seem to work, right. Instead of it displaying the message "Invalid value", it just shows "<input type="text" name="title" value="Invalid value."
[/quote]

That's because it terminates the function and returns "invalid value" If you want it to display "invalid value" [i]instead[/i] of the input tags, then you are going to have to wrap a condition around the input tags.  Something like this:

[code]
function getIt($value) {
.
.
.
}

$blah = $getIt('price');
if ($blah == "Invalid Value" || $blah == "") { // or whatever you want it to return when it fails
  echo $blah;
} else {
  // display your input tags
}
[/code]

[ironzero]

You're right, I don't want letters in the price field, so I guess one preg_match won't do then.

So, a custom expression for each field??

You mean like this:

if (!preg_match('/^[$0-9.]*$/',$_GET[$price]))

if (!preg_match('/^[0-9a-zA-Z]*$/',$_GET[$title]))


I'm just confused as to how I'm supposed to define $price and $title as variables.

Thanks!



[quote author=Crayon Violent link=topic=119591.msg490001#msg490001 date=1166761541]
[quote author=ironzero link=topic=119591.msg489996#msg489996 date=1166760881]
I see, so in other words it's easier to just use one preg_match that checks everything, right?
[/quote]
well...not necessarily.. I mean, it depends on how picky you are.  For instance, that preg_match makes sure only dollar signs, numbers and dots are passed.  That probably isn't what you want for your title, right?  But if you include letters, then having a single preg_match would allow people to put letters into the price, as well.  So unless you can live with that, you're gonna want to build a custom expression for each field. 

[quote]
Also, I tried using:

die("Invalid value.");

instead of:

return "";


...but it doesn't seem to work, right. Instead of it displaying the message "Invalid value", it just shows "<input type="text" name="title" value="Invalid value."
[/quote]

That's because it terminates the function and returns "invalid value" If you want it to display "invalid value" [i]instead[/i] of the input tags, then you are going to have to wrap a condition around the input tags.  Something like this:

[code]
function getIt($value) {
.
.
.
}

$blah = $getIt('price');
if ($blah == "Invalid Value" || $blah == "") { // or whatever you want it to return when it fails
   echo $blah;
} else {
   // display your input tags
}
[/code]


[/quote]

[trq]

Maybe something like...

[code]
<?php
function getIt($value){
  if($_GET[$value]) {
    switch ($value) {
      case 'price':
        $pattern = '/^[$0-9.]*$/';
        break;
      case 'title':
        $pattern = '/^[0-9a-zA-Z]*$/';
        break;
    }
    if (preg_match($pattern,$_GET[$value])) {
      return $_GET[$value];
    }
  }
  return false;
}
?>
[/code]

[ironzero]

[quote author=thorpe link=topic=119591.msg490010#msg490010 date=1166762155]
Maybe something like...

[code]
<?php
function getIt($value){
  if($_GET[$value]) {
    switch ($value) {
      case 'price':
        $pattern = '/^[$0-9.]*$/';
        break;
      case 'title':
        $pattern = '/^[0-9a-zA-Z]*$/';
        break;
    }
    if (preg_match($pattern,$_GET[$value])) {
      return $_GET[$value];
    }
  }
  return false;
}
?>
[/code]
[/quote]

Thanks, that seems to work quite nicely! :)

I'm just a little unclear with two other things...

When using [b]return "";[/b] or [b]return false;[/b] (which I assume is the same thing since they both return nothing, right?), is it just as secure as using say [b]die();[/b]?

If a user tried to pass some malicious code, would using [b]return "";[/b] or [b]return false;[/b] stop it? I would think that [b]die();[/b] would be better since it stops the script from executing, right?

Also, I tried adding {1,5} to preg_match to specify the min and max length, but it keeps giving me errors.

Thanks!

[trq]

Yes "" does equal false but its more readable to use false explicitly. Using die() within a function may not have the desired effect as it kills the script.

You will find most (if not all) native php functions return a result (or true) on sucess and false on failure. This makes using them seem more native. eg;

[code=php:0]
if (foo()) {
  // success
} else {
  // foo() failed.
}
[/code]

[ironzero]

[quote author=thorpe link=topic=119591.msg490048#msg490048 date=1166767711]
Yes "" does equal false but its more readable to use false explicitly. Using die() within a function may not have the desired effect as it kills the script.

You will find most (if not all) native php functions return a result (or true) on sucess and false on failure. This makes using them seem more native. eg;

[code=php:0]
if (foo()) {
  // success
} else {
  // foo() failed.
}
[/code]
[/quote]

I see, that makes sense.

So, as I take it the whole point of sanitizing input is to prevent malicious code from reaching the PHP parser by cleaning the input first, right?

So with that in mind, by returning a false value (using return false;), if an illegal/invalid character is encountered using preg_match, will the malicious code still be parsed? Or is return false; only for visual usage, so that the data isn't displayed on the page?

Thanks!

[trq]

[quote]will the malicious code still be parsed?[/quote]

People can post php code all day to your forms and it will not be parsed as php unless you send it to [url=http://php.net/eval]eval[/url] or some other dangerous function.

What your trying to do is prevent any malicious SQL getting to your database, and put simply, just because it doesn't pass the test doesn't meen it wont get there. Its up to you to stop it.

[.josh]

...which you don't really need to worry about sql injection if you aren't using your $_GET vars with a db.  It doesn't seem like that's what you are doing, so, no worries as far as sql injection.

[ironzero]

Nope, I'm not using SQL. :)

Basically, I'm just using this script for server side - that's it.

So, if I'm not using SQL, would you consider this script secure?

Wouldn't preg_match be enough to sanitize my input since I'm only allowing a few characters to be processed? I mean could someone exploit something with only $ . a-z 0-9 and maybe , -

Thanks guys!

[trq]

That really dpesnds on where your data is going doesn't it? But yeah, on the whole, I guess its [i]safe[/i].

[ironzero]

[quote author=thorpe link=topic=119591.msg490070#msg490070 date=1166774221]
That really dpesnds on where your data is going doesn't it? But yeah, on the whole, I guess its [i]safe[/i].
[/quote]

That's true, I guess nothing is ever 100% secure, right?

Is there anything you would do to make it more secure?

[code]
<?php
function getIt($value){
  if($_GET[$value]) {
    switch ($value) {
      case 'price':
        $pattern = '/^[$0-9.]*$/';
        break;
      case 'title':
        $pattern = '/^[0-9a-zA-Z]*$/';
        break;
    }
    if (preg_match($pattern,$_GET[$value])) {
      return $_GET[$value];
    }
  }
  return false;
}
?>
[/code]

Thanks.