Is this code secure enough for a minimum and maximum int

Question

So, I have created some code in php, and now I want to know if its "secure enough". This is the code:

$amount = $_POST['amount'];
if ($amount < 1) { die("Min amount is 1."); }
if ($amount > 20) { die("Max amount is 20."); }
// More stuff here

Is it possible, to for example somehow get the $amount set to 50, and still let it work? After this it will send a file_get_contents to a webpage with the number, for example: https://example.com/amount.php?a=15. Is it possible to get that 15 to be 50 at the "More stuff here" part.

I'm just curious, that's all.

EDITS

Here is the html Im using;

<form method="POST">
    <input type="number" class="form-control form-control-user" id="amount" name="amount" min="1" max="20" value="1"><br>
    <button class="btn btn-success" type="submit">send</button>
</form>

Then this is my full php (for example)

if (isset($_POST['amount'])) {
   $amount = $_POST['amount'];
   if ($amount < 1) { die("Min amount is 1."); }
   if ($amount > 20) { die("Max amount is 20."); }
   $conn->query("INSERT INTO my_table (amount) VALUES ('$amount')");
   file_get_contents("https://example.com/file.php?a=".$amount); // POINT A
}

Is it somehow possible to send at "POINT A" a other number? Like 50?

_Is it possible to get that 15 to be 50 at the "More stuff here" part._ Yes `$amount = 50;` or even `$amount += 35;` — AbraCadaver, Jun 21 '19 at 19:30
But I mean by `$_POST['amount']` if users can somehow put something in the number input place, and then get it to 50, and bypassing those 2 if thingies that check if its under 1 and above 20. — Aaron Jonk, Jun 21 '19 at 19:31
Do you expect to handle fractions of an item, like `1.5`?, also nothing is going to work for `?a=15` as your reading from `$_POST['amount']` not `$_GET['a']` — Lawrence Cherone, Jun 21 '19 at 19:37
It sends at `// More stuff here` the request to `https://example.com/amount.php?a=$amount` with some more variables added in. I'm asking if they can somehow make the `$amount` variable higher then 20, and still let the script send the request. — Aaron Jonk, Jun 21 '19 at 19:44
no it's not, but if you change the second line to `$amount = (float)$_POST['amount'];` then it would be sufficient :) — hanshenrik, Jun 21 '19 at 20:38

MonkeyZeus · Answer 1 · 2019-06-21T19:49:58.287

4

You need to:

$amount = (int)$_POST['amount'];

or else the user can enter something such as:

3*3

which can cause issues for your file_get_contents( 'https://example.com/amount.php?a=3*3' )

A truly problematic scenario would be this input:

7&a=8734

which would send the request as file_get_contents( 'https://example.com/amount.php?a=7&a=8734' )

The second a's value would win so the example.com server would interpret your request as if you had sent file_get_contents( 'https://example.com/amount.php?a=8734' )

This "attack" works because of PHP's type-juggling and that it converts 7&a=8734 into simply 7 for purposes of the comparison but you are sending the non-juggled version on to example.com.

This is why $amount = (int)$_POST['amount']; is the proper solution.

edited Jun 21 '19 at 19:49

answered Jun 21 '19 at 19:41

MonkeyZeus

20,375
4
36
77

My question more was like: It sends at `// More stuff here` the request to `https://example.com/amount.php?a=$amount` with some more variables added in. I'm asking if they can somehow make the `$amount` variable higher then 20, and still let the script send the request. – Aaron Jonk Jun 21 '19 at 19:44
@AaronJonk Yes, I saw it. `min="1" max="20"` won't save you because it is super trivial to open up the web browser's console and remove those. HTML is client-side so there is zero security to be had. Your server accepts `$_POST['amount']` so you need to properly handle that value on the server-side. – MonkeyZeus Jun 21 '19 at 19:55
But its nice for the people to know that there is a maximum of 20. (As I am using bootstrap, and it will pop up something if you try to get over it). It's more about if its possible to do harm to my website. – Aaron Jonk Jun 21 '19 at 19:57
@AaronJonk Okay, so what part of my answer is confusing? Simply put, your code is open for pwnage as it stands. – MonkeyZeus Jun 21 '19 at 19:57
But I would like to know how it would to be possible to do any harm to this exact code – Aaron Jonk Jun 21 '19 at 20:00
I explained the harm for `file_get_contents()`. Now that you've added SQL to your question then you need to check out https://stackoverflow.com/q/60174/2191572 because there is zero reason for me to continue explaining anything. One day when your system is hacked then feel free to re-read my answer. – MonkeyZeus Jun 21 '19 at 20:03
do we know amount can't be float? otherwise you might want to cast it to (float) instead of (int) – hanshenrik Jun 21 '19 at 20:39
@hanshenrik OP's HTML does not make use of the `step` attribute so I think it is safe to assume whole numbers. – MonkeyZeus Jun 24 '19 at 12:16

Don't Panic · Answer 2 · 2019-06-21T20:56:23.957

One example of how your code is problematic:

By posting a value like this:

1'),('50'),('42

a user can insert multiple values into my_table, any values they want to try.

That string is >= 1, and <= 20.

It's simple to defeat client-side validation, it really isn't even necessary to use a browser to post values to your script.

You need to bind that value to a prepared statement instead of including it directly in the SQL string to prevent this type of mischief.

$stmt = $conn->prepare("INSERT INTO my_table (amount) VALUES (?)");
$stmt->execute([$amount]);

You must do this with any value you need to use in a query. Don't depend on filtering, escaping, or casting. It's not that they can't work, it's just that if you're in the habit of using variables directly in your query like INSERT ... VALUES ('$amount') you depend on your ability to remember to filter/escape/cast 100% of the time, and we're only human.

score 1 · Answer 3 · answered Jun 21 '19 at 19:45

1

Never trust user input

That in and of itself is not secure. Users can manipulate your forms via inspection tools or post anything they want with post man. A ! 1 3.5. All are viable inputs. This also includes XSS, SQL injection, etc etc.

That said you need to sanitize your inputs first. I would suggest using the is_numeric function first then dying if it returns false. After that you know then it's in fact a number which you can check the range on.

if(!is_numeric($amount))
    die();

answered Jun 21 '19 at 19:45

Lulceltech

1,662
11
22

Is it then possible that if I put the input to, for example; `echo "hi";` it will actually do it? I want to know how it would be able to get bypassed.. – Aaron Jonk Jun 21 '19 at 19:49
It is possible, though XSS is a bigger concern than echoing something. Always sanitize your data. strip_slashes is good but using a preg_replace with a good pattern is the best way to sanitize it. – Lulceltech Jun 21 '19 at 19:51
Yes it's possible still. – Lulceltech Jun 21 '19 at 19:56

hanshenrik · Answer 4 · 2019-06-21T20:50:16.033

the other answers are excellent in explaining why it's not sufficient, but none show how to properly validate the input, which can be done with filter_var() and would look something like:

if (isset($_POST['amount'])) {
   $amount = $_POST['amount'];
   if(false===($amount=filter_var($amount,FILTER_VALIDATE_FLOAT)){
      http_response_code(400); // HTTP 400 Bad Request
      die("invalid amount (not a number)"); // show error page
   }
   // $amount is guaranteed to be a valid floating-point number,
   // also filter_var() did the typecast for us,
   // so $amount is now a php-native float.
   if ($amount < 1) { die("Min amount is 1."); }
   if ($amount > 20) { die("Max amount is 20."); }
   $conn->query("INSERT INTO my_table (amount) VALUES ('$amount')");
   file_get_contents("https://example.com/file.php?a=".$amount); // POINT A
}

if $amount can only be integers, not floats, then use FILTER_VALIDATE_INT instead of FILTER_VALIDATE_FLOAT

Is this code secure enough for a minimum and maximum int

4 Answers4