Secure a class for an hydratation from ma form POST

Question

I use PHP classes for managing users accounts and I wonder if what I'm doing is correct.

I directly use the $_POST['form'] for hydrate User objects. But there is some field that I don't want user to modify (i.e. : Id_user, admin_level, ... [They can be able to do it by creating a new input field called id_user or admin_level, and get admin level])

So I use an argument in each setID or setAdmin_level (a boolean called $forcer) :

<?php
Class User extends Content{
private $_id_user;
private $_date_inscription;
private $_ip_inscription;
private $_derniere_connexion;
private $_nom_utilisateur;
private $_email;
private $_mot_de_passe;
private $_nom;
private $_prenom;
private $_role;

const USER_UNLOGGED = 0;
const USER_LOGGED = 1;
const USER_ADMIN = 5;

public function __construct(array $donnees = null, $forcer = false)
{
    if($donnees){
        $this->hydrate($donnees,$forcer);
    }
}

public function hydrate(array $donnees, $forcer = false)
{
    foreach($donnees as $champ => $valeur){
        $method = 'set'.ucfirst($champ);
        if(method_exists($this,$method))
        {
            if($forcer){
                try {
                    $this->$method($this->securite($valeur), true);
                }catch(Exception $e){
                    $this->$method($this->securite($valeur));
                }
            }else {
                $this->$method($this->securite($valeur));
            }
        }
    }
}

public function setId_user($id_user, $forcer = false)
{
  if(is_numeric($id_user)&&$forcer)
  {
      $this->_id_user = $id_user;
      return true;
  }else {
    $this->addErreur('id_user','User ID incorrect');
    return false;
  }

}

public function getId_user()
{
    return $this->_id_user;
}

public function setDate_inscription($date_inscription = "")
{
  if(is_numeric($date_inscription))
  {
      $this->_date_inscription = $date_inscription;
  }else {
      $this->_date_inscription = time();
  }

}

public function getDate_inscription()
{
    return $this->_date_inscription;
}

public function setIp_inscription($ip_inscription ='')
{
  if($ip_inscription)
  {
      $this->_ip_inscription = $ip_inscription;
  }else {
      $this->_ip_inscription = $_SERVER['REMOTE_ADDR'];
  }
}

public function getIp_inscription()
{
    return $this->_ip_inscription;
}

public function setDerniere_connexion()
{
  $this->_derniere_connexion = time()."#".$_SERVER['REMOTE_ADDR'];
}

public function getDerniere_connexion()
{
    return $this->_derniere_connexion;
}

public function setNom_utilisateur($nom_utilisateur)
{
    $this->_nom_utilisateur = $nom_utilisateur;
}

public function getNom_utilisateur()
{
    return $this->_nom_utilisateur;
}

public function setEmail($email)
{
  if($this->is_mail($email))
  {
      $this->_email = $email;
  }else {
      $this->addErreur('email','email incorrect.');
      return false;
  }
}

public function getEmail()
{
    return $this->_email;
}

public function setMot_de_passe($mot_de_passe, $encrypted=false)
{
  if($this->is_password($mot_de_passe))
  {
    if($encrypted)
    {
      $this->_mot_de_passe = $mot_de_passe;
    }else {
      $this->_mot_de_passe = crypt($mot_de_passe, $GLOBALS['salt_crypt']);
    }
  }else{
    $this->addErreur('mot_de_passe','Mot de passe incorrect. Minimum 6 caractères.');
    return false;
  }

}

public function getMot_de_passe()
{
    return $this->_mot_de_passe;
}

public function setNom($nom)
{
    $this->_nom = $nom;
}

public function getNom()
{
    return $this->_nom;
}

public function setPrenom($prenom)
{
    $this->_prenom = $prenom;
}

public function getPrenom()
{
    return $this->_prenom;
}

public function setRole($role, $forcer = false)
{
  if(is_numeric($role)&&$forcer)
  {
      $this->_role = intval($role);
  }else{
    $this->addErreur('role','Role incorrect');
    return false;
  }

}

public function getRole()
{
    return $this->_role;
}
} // Fin de la classe User

So, in order to register a new user, I hydrate a new User object with the form POST :

$user = new User($_POST['form'], false);

And I need to set the $force bool to true for create a new User from an Id or if I want to set a field protected.

$user = $userManager->getUserFromId(new User(['id_user' => 1], true));

Is this a good way?

Answer 1

ALWAYS sanitize and filter user input FIRST

I have to agree with mickmackusa that using French in programming, for variables names or comments, is not a very good idea. For one, it makes it harder to discuss your code here. But also: Most codes and tutorials are in English. It also just looks weird: getMot_de_passe should either be obtenir_mot_de_pass or getPassword. I prefer the latter. (FYI: I'm Dutch).

It is clear what you try to achieve with your code, but I do wonder about how secure it is. Two points:

1. The risk. I was always taught that user input should be strictly sanitized and filtered as soon as possible, before it is allowed to enter deeper into the code. In your code you hand over the raw $_POST array to an User object, and then, without anything else, the hydrate() method hands the content over to another method, and so on. We are now deep into this code, and can you be certain you will always properly sanitize and filter user input? Will you think about this, in a years time, when you want to extend the User class? If anything, this is just dangerous.

2. The hole. Your hydrate() method nicely checks if a method exists for a given $_POST key. So 'id_user' finds setId_user(), as it should, but 'hack_user' cannot find setHack_user() because that method doesn't exist. The user could however change the registration date: 'date_inscription' will be connected to the setDate_inscription() method. There are more methods like this. I'll admit that the user needs to know the names of these methods, but security through obscurity is always a bad plan. More so, these methods lack the minimal sanitizing and filtering you have in the other methods. So this is clearly a security hole. You have given the user access to the control structures of your program, probably without even realizing it.

So, my suggestions would be: First sanitize and filter user input, before it gets too deep into your code and you loose sight of all the security implications. This would, in one fell swoop, also prevent the security hole I mentioned in my second point.

Personally I don't like the hydrate() method. I think it is ugly. It will be difficult to predict what it does, to check its security, and it makes proper debugging virtually impossible.

To end with a tiny bit of code. Here is how I would sanitize and filter user input. Put this code at the top of the PHP file to which your form is posted.

<?php

$post_userId = filter_input(INPUT_POST, 'userId', FILTER_SANITIZE_NUMBER_INT);
$post_roleId = filter_input(INPUT_POST, 'roleId', FILTER_SANITIZE_NUMBER_INT);

Don't worry that this is not OOP. That is NOT important. Safety first! The prefix post_ means that this content comes from the user. This clearly distinguishes them from other variables. Other solutions are possible, and an array is allowed, of course. Further filtering should be done, if possible. For instance if you know that a role id should be between 1 and 5 then anything outside that range should be invalid, and not used. In that case either set it to its default value, or generate an error.

Now, and only after you've applied all the sanitizing and filtering your can think of, should you allow the input from the user to touch your code.

Answer 2

Just some minor suggestions...

• I think it is the general advice (from what I've read) that you should use English variables. This seems horribly biased from a coder who only speaks English, but I have seen many English As A Second Language developers post this same advice. I took just enough French in high school to follow your variable names.

• All of the lines inside your class should be tabbed in.

• If you are expecting all of the numeric values to be integers, you could implement the more strict function ctype_digit() to validate.

• The use of password_hash() is encouraged. https://www.php.net/manual/en/function.crypt.php