DEV Community

loading...

PHP Validating a form using PHP

Jorge Castro
You are free to believe in whatever you want to, me too. So, stop preaching your religion, politics, or belief. Do you have facts? Then I will listen. Do you have a personal belief? Sorry but no.
Updated on ・1 min read

Discussion (10)

Collapse
lawrencejohnson profile image
Lawrence

Sadly, this article doesn't teach anything about the fundamentals of processing form data. Sure it's good to leverage libraries to avoid reinventing the wheel, but failure to explain things like filtering http headers is why you see so many answers on SO accessing $_POST directly.

Collapse
jorgecc profile image
Jorge Castro Author • Edited

Yes but it simplifies the use of a form.

For example, let's say a simple form with a simple field "id":

$id=$_POST['id'];
echo $id+20;

Now, what if the user enters value that it's not a number or what if "Id" is empty. It will generate a warning if not an error.

A proper way is:

$id=isset($_POST['id'])?$_POST['id']:0;
if (!is_numeric($id)) {
   $id=0;
   $messageId='id is not numeric';
} else {
   $messageId='';
}

echo $id+20;
echo $messageId;

And it is a short and minimalist but I am not doing any special but collecting from the POST (and validating the information).

Or you could use this library:

$id=$validaton->type('integer')->def(0)->post('id');

echo $id+20;
echo $validaton->getMessageId('id')->firstError();

Now, what if the form has +10 fields?. And what if there is more than one form per instance. For example we have a CRUD and there is a form for create, replace, edit and delete.

Now, let's add a simple logic, "id" must be lower than 10.

Vanilla PHP:

$id=isset($_POST['id'])?$_POST['id']:0;
if (!is_numeric($id)) {
   $id=0;
   $messageId='id is not numeric';
} else {
   $messageId='';
   if ($id>=10) {
       $messageId='Id must be less than 10';
   }
}

echo $id+20;
echo $messageId;

Using the library:

$id=$validaton->type('integer')->def(0)
     ->condition('lt','Id must be less than 10',10)
     ->post('id');

echo $id+20;
echo $validaton->getMessageId('id')->firstError();

It depends on the type (and size) of the form. If the form is small then we could use vanilla PHP and it will work fine but if the project is taking size, then it's not as simple and maybe we will need to add a validation class. And this validation is per-form-basis

For example, let's say we create a validation method to validate an object.

function validate($user) {
  // here our logic and returns true/false or a message
}

However, this method is not based on the form but in the model (user). If we are creating a user, we need the user and password, however, if we are editing a user, then we need to enter other information, maybe the password is not required.

Collapse
lawrencejohnson profile image
Lawrence

None of the methods you pose, including a quick glance at the source of your library (I might've overlooked it), actually prevent malicious user data.

If you want to make sure you are dealing with sanitized user input, you should always use some type of filtration method for retrieving http header data. PHP provides this in a very easy use to fashion that while it does not cover error messaging, can easily handle most other things.

$name = filter_input(INPUT_POST, 'name', FILTER_SANITIZE_STRING);
if ($name === null) {
   // Key not supplied in HTTP Post headers
} else if ($name === false) {
   // Value could not be sanitized, could be from malicious input
} else if (strlen($name) === 0) {
   // Did not meet required parameter
}

$email = filter_input(INPUT_POST, 'email', FILTER_VALIDATE_EMAIL);
if ($email === null) {
   // key not supplied
} else if ($email === false) {
   // email is invalid
}

$product_id = filter_input(INPUT_POST, 'prodid', FILTER_SANITIZE_NUMBER_INT);
// etc, etc. There are many more options.

As noted previously, I'm not saying it's a bad idea to use a library. What is bad is that the article is presented as a "how to", but is not teaching anything about fundamentally important rules for capturing form data. It would be like having an article on how to encrypt passwords but only pointing to a library that handles everything for you. What happens when the developer needs to work in an environment that doesn't use this library?

Back to the point of this library, I would be curious to know if anyone has even checked to see if it does anything to clean malicious user input. Which would mean on top of not teaching developers the proper methods for secure form functionality, the author would also be pointing devs to blindly use a library that has security holes.

Thread Thread
jorgecc profile image
Jorge Castro Author

tsk tsk

$name = filter_input(INPUT_POST, 'name', FILTER_SANITIZE_STRING);

You are changing the input. How many forms do that?.

Let's say $_POST['mail']='email@gmail.com******' and our code is:

$mail= filter_input(INPUT_POST, 'mail', FILTER_SANITIZE_EMAIL);
// ....
echo "This email [$mail] is incorrect";

This code will show the next message

This email [email@gmail.com] is incorrect

However, the email "email@gmail.com" is correct (it is because filter deleted all the extra "*" but the user doesn't see it).

A proper system would show the next message

This email [email@gmail.com********] is incorrect

Anyways, this library allows to add custom validations but it's up to the developer to do the validations. Email are common but they aren't unique, for example SSN, phone, address, product code, ISBN and such. Html (url) is anything but common and IP is really niche. This library is not aimed to validate all and every possible situation, simply because it is not realistic.

But let's say about email:

$email=$validation->type('varchar')
      ->condition('fn.global.isEmail','the field is not an email')
      ->post('email');

function isEmail($value,$comparevalue) {
    if (!filter_var($value, FILTER_VALIDATE_EMAIL)) {
        return false;
    } else {
        return true;
    }
}

where fn.global.functionname is a call to a global function.

But this example can be improved.

  • If we use SQL at some point (what if we are storing this value), then we must set a maximum length and if we use NO-SQL then we want to limit it regardless.

  • Also, what if we want to limit the domains of the email?, we don't want free emails.

  • Also, what if the email must be unique and it mustn't be on the database?.

$email=$validation->type('varchar')
      ->condition('fn.global.isEmail','the field is not an email')
      ->condition('fn.global.emailDomain','This email is not allowed')
      ->condition('fn.static.ClassDao.emailUsed','This email already exists on the database')
      ->condition('maxlen','The maximum size of the field is 50',50)
      ->post('email');

function isEmail($value,$comparevalue='') ....
function emailDomain($value,$comparevalue='') ...

class ClassDao {
  public static emailUsed(...) ...
}

where fn.static.classname.functionname is a call to a static function inside classname.

So this library has the flexibility to create your own validations and still be integrated and executed on the validation pipeline.

Thread Thread
lawrencejohnson profile image
Lawrence • Edited

There's nothing to stop you from doing additional validation AFTER you have filtered it. You have made a very unwise choice to allow users to freely submit any form data to your code. Regardless of what you do before sending it to SQL, there are far more security implications you are completely ignoring by assuming you can just deal with that later.

Honestly, your library is fine, it's nice really. All you need to do is filter your inputs before you do all of your validation functionality. Fighting it is only going to bite you later when you have to work for an organization that cares about security.

All of this goes back to my original point that this how-to guide is only a "How to Use My Library" guide. This doesn't teach new developers a very important aspect of allowing users to submit content to your application: security.

Collapse
tarialfaro profile image
Tari R. Alfaro • Edited

In some cases getimagesize function doesn't work and I could save a PHP file as anyfilename.jpg (rendering the file extension validation useless).

I think if you change properties on the file and add dimension values to the file it could bypass the getimagesize function.

As far as I know, securing file uploads is pretty difficult.

Collapse
tarialfaro profile image
Tari R. Alfaro • Edited

If you want to see a "bug", there is a link for you. I just did a little more research on the subject. Also it's not actually a bug because getimagesize is doing exactly what it's supposed to be doing.

The issue is, is that developers are relying on it to validate images when it IS NOT ideal for doing so.

bugs.php.net/bug.php?id=69002&edit=1

getimagesize() merely provides information about an image. It does not, nor does it claim to, perform anything resembling validation beyond what is strictly required to derive the information it seeks.
Anyone relying on it for complete validation of a file is abusing it at their own risk.

Collapse
jorgecc profile image
Jorge Castro Author • Edited

That vulnerability was patched years ago.

I tested with the malicious gif it allows to upload the file. However, it doesn't trigger any malicious.

https://i.ibb.co/0MJk18h/craftedgif.jpg

Getimagesize doesn't check if the file is corrupt, malicious or not, it simply checks the headers. if we need that, then we should use an external service or library.

The usual security is to convert all images into a jpg or png but there are more vulnerabilities around it.

Thread Thread
tarialfaro profile image
Tari R. Alfaro

That's good to know.

Collapse
azazqadir profile image
Muhammad Azaz Qadir

Why someone would add validation manually when there are so many PHP validation libraries available I wonder? Businesses don't have the cost or time to do this. An individual might have though.

Forem Open with the Forem app