password could get inserted as plaintext when adding a user

[louis123562]

Hey,

i think there is a missing check on the "hashed" password in function 'addUser()', if the password is really hashed.

On my server, there was not enough RAM available for executing another process (in this case - i guess - hashing the pw), somehow the password of the new user was stored in plain-text in phpauth_users. I was lucky and mentioned it very fast...

At least there should be something like

//$password = $this->getHash($password);
$plain_pw = $password;
$hashed_pw= $this->getHash($password);

//basic string compare
if ($plain_pw === $hashed_pw){
   $return['error'] = true;
   $return['message'] = "Password could not be hashed"; //For a end-user of course a simpler explanation is needed
    return $return;
}
....
$query = "UPDATE {$this->config->table_users}............";
....

What do you think? That my server had not enough memory was my fault for sure, but is a case that could happen.

[KarelWintersky]

NEVER compare strings with ==

Write your server configuration.

Also, need more informiation. Is this problem repeatable?

[louis123562]

phpinfo() enough (attached)? What else do you want to know about the server? It is a virtual one via KVM.

From the Serverlog; how the process with the biggest amount of RAM was killed (clamd / clamAV):

Aug 24 16:01:08 server kernel: [4368128.572765] Out of memory: Kill process 364 (clamd) score 273 or sacrifice child
Aug 24 16:01:08 server kernel: [4368128.573289] Killed process 364 (clamd) total-vm:1329368kB, anon-rss:1102956kB, file-rss:0kB, shmem-rss:0kB
Aug 24 16:01:08 server kernel: [4368128.585608] oom_reaper: reaped process 364 (clamd), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB

Seems to be a normal behaviour on Linux to kill the process with the highest RAM usage.
Our server did reach the memory limit multiple times and because clamd/clamAV takes about 1.3gb of RAM, there was always enough RAM after the process "killed itself".

We came up on to this, when we saw this SMTP error, because a customer did not get his mail with login-information from phpAuth.

SMTP Error: data not accepted.SMTP server error: DATA END command failed Detail: Service unavailable - try again later
SMTP code: 451 Additional SMTP info: 4.7.1

The mail-service was also killed / did not have enough RAM to operate correctly. Because the PHPAuth user was stored in the database already and the mail was not send, we checked the server log and also the user itself in phpauth_users. Because the customer knew its password, i was confused when i saw it plain in the db...

I did not try to reproduce this problem yet - but it happened.

[KarelWintersky]

I'll think about your problem.

I'll implement this in V2 PHPAuth... later. Thank you.

[Conver]

@KarelWintersky V2?

[louis123562]

Okay - i found out, that the inserted plain-text password was inserted by myself via updateUser() and i forgot to hash the password in a very rare case......

Because of this, i would also suggest to implement a check in updateUser(), if params contains password:

//Is there a password present in $params?
if(isset($params['password']))
{
   //Is the password hashed?
   if(!is_hashed($params['password'])){
        $return['error'] = true;
        $return['message'] = 'User could not be updatet: ...';
        return $return;
    }
}

$query = "UPDATE............

[KarelWintersky]

@Conver , yeah. Fluent config, more exceptions etc.
In progress.

[Conver]

@KarelWintersky interesting,let me know if you need help.

[Conver]

@louis123562 - I can add this next week 👍

[KarelWintersky]

interesting,let me know if you need help.

@Conver , I really need more hours in a day. ;-)