Am I sufficiently cleaning incoming $_GET parameters, especially in the array?

Question

I'm trying to clean some incoming $_GET parameters. I've not done this before, so I'd love feedback.

I'm especially concerned with the array. While all the other parameters control simple logic, the array will be saved into the database and potentially output to users.

Please also feel free to point out any redundancies.

<?php
// int: only want a positive value
// array: wont know what this could be but it could be anything urlencoded
// string: three possible outcomes: foo, bar, fudge
( isset( $_GET["val1"] ) ? (bool) $val1 = true : (unset) $val1 );
( isset( $_GET["val2"] ) ? (int) $val2 = sanitize_absint( $_GET["val2"] ) : (unset) $val2 );
( isset( $_GET["val3"] ) ? (array) $val3 = sanitize_array($_GET["val3"]) : (unset) $val3 );
( isset( $_GET["val4"] ) ? (string) $val4 = sanitize_string($_GET["val4"]) : (unset) $val4 );
// further refine val4 string
val4_strip($val_4);
// sanitize
function gfahp_sanitize_absint( $int ) {
 $int = filter_var( $int, FILTER_SANITIZE_NUMBER_INT );
 $int = abs( intval( $int ) ); // positive number
 return $int;
}
function gfahp_sanitize_string( $string ) {
 // is this enough to prevent anything icky?
 $string = filter_var( $string, FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW );
 return $string;
}
function gfahp_sanitize_array( $array ) {
 $array = array_walk_recursive( $array, "gfahp_sanitize_string" );
 return $array;
}
function val4_strip($val_4){
// strip down val4
 if (!empty($val4){
 switch ($val4) {
 case 'foo':
 $val4 = 'foo';
 break;
 case 'bar':
 $val4 = 'bar';
 break;
 case 'fudge':
 $val4 = 'fudge';
 break; 
 default:
 $val4 = (unset) $val4
 break;
 }
 }
 return $val4;
}

Answer 1

My 2 cents:

$int = abs( intval( $int ) ); // positive number

Silently fixing input values is evil, perhaps required, but still evil.

function gfahp_sanitize_array( $array ) {
 $array = array_walk_recursive( $array, "gfahp_sanitize_string" );
 return $array;
}

You assign and then return, I would just return immediately

function gfahp_sanitize_array( $array ) 
{
 return array_walk_recursive( $array, "gfahp_sanitize_string" );
}

Also, what is up with gfahp ? Hungarian notation of functions with acronyms is a slappable offence.

As for

function val4_strip($val_4){
// strip down val4
 if (!empty($val4){
 switch ($val4) {
 case 'foo':
 $val4 = 'foo';
 break;
 case 'bar':
 $val4 = 'bar';
 break;
 case 'fudge':
 $val4 = 'fudge';
 break; 
 default:
 $val4 = (unset) $val4
 break;
 }
 }
}

You should probably go for a generic function that can look up a value in a set and assign it if found, otherwise return blank. ( Also that function does not return anything? ) (Also you are unsetting var4 in places ). Something like

$val4_set = array( "foo" , "bar" , "fudge" );
( isset( $_GET["val4"] ) ? (string) $val4 = sanitize_string_from_set($_GET["val4"], $val4_set) : (unset) $val4 );
function sanitize_string_from_set( $string , $set )
{
 return array_key_exists( $string, $set )?$string:""; 
}

• orionrush: Hungarian?, slapable? buuuuhahahahahah -- sorry I may have passed my Balmer peak. Wonderfull feedback truly many thanks!
• orionrush: As for $int = abs( intval( $int ) ); how would you do it differently? Mind you someone could try to submit a neg number and it would still fail.
• konijn: I would catch it in the UI before submitting, if someone tries to hack I would check whether the value < 0 and throw an error in that case.
• orionrush: good point re <0. No ui on this, its a service that returns GravityForms output via get params. Im just tring to keep script kiddies from imputing undeseriable XSS.
• orionrush: Oh just to round trip your comments re Hungairian Notaion, those are actually function prefixes - which are a good thing, no?