CODEWITHSUNDEEP
phpvalidationpost
m1xolyd1an
m1xolyd1an

Reputation: 535

Validating POST variables in PHP

I have a section of code where the user completes an input form in HTML. On post I validate their $_POST entry $userAmt against their points available $points in the database and then I proceed.

I've noticed that some users are able to circumvent my initial check and enter more points than are available in the database only as long as it's within the range of the second validation 1-20. So some part of the validation is working, but I'm guessing they are able to use a break point and edit the post variables after my initial validation? Here's a snippet of the code. 

$query = mysqli_query($conn, "SELECT * FROM users WHERE id = '$id'");
$queryA = mysqli_fetch_assoc($query);
$points = $queryA['points']; //user points available


if(isset($_POST['submit'])){
$userAmt = $_POST['userInput']; //users input
$userAmt = mysqli_real_escape_string($conn, $userAmt);
$prize = $userAmt * 2;

    if($userAmt > $points ){ //<- attackers are circumventing
    $message = "You do not have enough points";
    } else if ($userAmt < 1 || $userAmt > 20){ //<-attackers are not circumventing
    $message = "Must be between 1 and 20";
    } else {
    // determine outcome and update DB
    // if user wins
    mysqli_query($conn, "UPDATE users SET points = points + '$prize' WHERE id = '$id'");
    }
}

Maybe I shouldn't be doing else if's and instead case switch?

Upvotes: 1

Views: 289

Answers (2)

Steve
Steve

Reputation: 20469

Its down to phps type juggling, and string comparison:

//your code, valunerable:
$userAmt = " 20 ";
$points = "10";

if($userAmt > $points ){ //<- attackers are circumventing
    $message = "You do not have enough points";
} else if ($userAmt < 1 || $userAmt > 20){ //<-attackers are not circumventing
    $message = "Must be between 1 and 20";
} else {
     $message = "winner";
}

You can avoid this by simply casting to int:

echo $message;
//fixed by type cast to int

$userAmt = (int) " 20 ";
$points = (int) "10";

if($userAmt > $points ){ //<- attackers are circumventing
    $message = "You do not have enough points";
} else if ($userAmt < 1 || $userAmt > 20){ //<-attackers are not circumventing
    $message = "Must be between 1 and 20";
} else {
     $message = "winner";
}

echo $message;

Live example: http://codepad.viper-7.com/bwkvG3

Upvotes: 2

Marc B
Marc B

Reputation: 360702

Cargo-cult programming. Why are you sql escaping $userAmt, only to start doing math on it?

You're also simply ASSUMING that your select query succeeds. if it fails for whatever reason, then$points will be a boolean FALSE value, which type-casts to an integer 0 when used in > operations, basically

if ($userAmt > false)

is essentially going to be true, always, as long the user enters ANYTHING.

You need something more like this:

$result = mysqli_query($con, "SELECT ...") or die(mysqli_error($conn));
if (mysqli_num_rows($result) == 0) {
   die("who are you and how did you get here?");
} else {
   $row = mysqli_fetch_assoc($result);
   $points = $row['points'];
}

And beyond this, you shouldn't be doing your own escaping anyways. You're using mysqli - use prepared statements and placeholders, because you're still vulnerable to sql injection attacks.

Upvotes: 5

Related Questions