[GH-ISSUE #71] Why are we storing the password in plain text AND encrypting it? #54

Closed
opened 2026-03-02 15:55:18 +03:00 by kerem · 17 comments
Owner

Originally created by @ghost on GitHub (Oct 17, 2018).
Original GitHub issue: https://github.com/prasathmani/tinyfilemanager/issues/71

// Users: array('Username' => 'Password', 'Username2' => 'Password2', ...)
$auth_users = array(
    'admin' => 'admin',
    'user' => '12345',
    'admin' => password_hash('admin', PASSWORD_DEFAULT),
    'user' => password_hash('12345', PASSWORD_DEFAULT)
);

This adds no additional security at all, we need a setup where the user passes the information through a post request, THEN it gets encrypted... of course, looking at this snippet, later we use password_verify, this does nothing to help us

Originally created by @ghost on GitHub (Oct 17, 2018). Original GitHub issue: https://github.com/prasathmani/tinyfilemanager/issues/71 ``` // Users: array('Username' => 'Password', 'Username2' => 'Password2', ...) $auth_users = array( 'admin' => 'admin', 'user' => '12345', 'admin' => password_hash('admin', PASSWORD_DEFAULT), 'user' => password_hash('12345', PASSWORD_DEFAULT) ); ``` This adds no additional security at all, we need a setup where the user passes the information through a post request, THEN it gets encrypted... of course, looking at this snippet, later we use password_verify, this does nothing to help us
kerem 2026-03-02 15:55:18 +03:00
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

The password is encrypted, please read the manual... http://php.net/manual/en/function.password-hash.php

'admin' => 'admin',
'user' => '12345',

no longer exists... now only passwords are encrypted... the user don't need to be encrypted... a security hole has been fixed...

// Users: array('Username' => 'Password', 'Username2' => 'Password2', ...)
$auth_users = array(
    'admin' => password_hash('admin', PASSWORD_DEFAULT),
    'user' => password_hash('12345', PASSWORD_DEFAULT)
);

Download the filemanger and read the source!

<!-- gh-comment-id:430631776 --> @alecos71 commented on GitHub (Oct 17, 2018): The password is encrypted, please read the manual... http://php.net/manual/en/function.password-hash.php ``` 'admin' => 'admin', 'user' => '12345', ``` no longer exists... now only passwords are encrypted... the user don't need to be encrypted... a security hole has been fixed... ``` // Users: array('Username' => 'Password', 'Username2' => 'Password2', ...) $auth_users = array( 'admin' => password_hash('admin', PASSWORD_DEFAULT), 'user' => password_hash('12345', PASSWORD_DEFAULT) ); ``` Download the filemanger and read the source!
Author
Owner

@ghost commented on GitHub (Oct 17, 2018):

yes but we are encrypting the password in the same file, we encrypt things so people cannot view the password, but the password is still in plain text, even though it gets encrypted on the same file, this adds no extra security at all, say the server was to get hacked, we would see the users password, passing it through encryption and using password verify does nothing though, we could do the same thing with the plain text password and it would be the same

<!-- gh-comment-id:430675803 --> @ghost commented on GitHub (Oct 17, 2018): yes but we are encrypting the password in the same file, we encrypt things so people cannot view the password, but the password is still in plain text, even though it gets encrypted on the same file, this adds no extra security at all, say the server was to get hacked, we would see the users password, passing it through encryption and using password verify does nothing though, we could do the same thing with the plain text password and it would be the same
Author
Owner

@hpolonkoev commented on GitHub (Oct 17, 2018):

@saderror256
The purpose of the library to be a single file without any database independences (or files with stored data). Therefore the password are stored directly in the file. If you don't want to have your passwords in a plain text, you can hash them beforehand (separatly somewhere else using the password_hash()) and just add a hash to the array.

$auth_users = array(
    'admin' => ADMIN HASH,
    'user' => USER HASH
);
<!-- gh-comment-id:430695111 --> @hpolonkoev commented on GitHub (Oct 17, 2018): @saderror256 The purpose of the library to be a single file without any database independences (or files with stored data). Therefore the password are stored directly in the file. If you don't want to have your passwords in a plain text, you can hash them beforehand (separatly somewhere else using the `password_hash()`) and just add a hash to the array. ``` $auth_users = array( 'admin' => ADMIN HASH, 'user' => USER HASH ); ```
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

@saderror256
The purpose of the library to be a single file without any database independences (or files with stored data). Therefore the password are stored directly in the file. If you don't want to have your passwords in a plain text, you can hash them beforehand (separatly somewhere else using the password_hash()) and just add a hash to the array.

$auth_users = array(
    'admin' => ADMIN HASH,
    'user' => USER HASH
);

There is no way to have encrypted the password in plain text, password_hash and password_verify are used to prevent that the password is sent in clear, when encrypted, the password cannot be hacked in the form login... read the manual before writing: http://php.net/manual/en/function.password-hash.php and http://php.net/manual/en/function.password-verify.php. If the password will be served encrypted in plain text php file would be impossible to change the password because people don't have the chance to modify the password... why do not you read the php manual before writing?

<!-- gh-comment-id:430711428 --> @alecos71 commented on GitHub (Oct 17, 2018): > > > @saderror256 > The purpose of the library to be a single file without any database independences (or files with stored data). Therefore the password are stored directly in the file. If you don't want to have your passwords in a plain text, you can hash them beforehand (separatly somewhere else using the `password_hash()`) and just add a hash to the array. > > ``` > $auth_users = array( > 'admin' => ADMIN HASH, > 'user' => USER HASH > ); > ``` There is no way to have encrypted the password in plain text, password_hash and password_verify are used to prevent that the password is sent in clear, when encrypted, the password cannot be hacked in the form login... read the manual before writing: http://php.net/manual/en/function.password-hash.php and http://php.net/manual/en/function.password-verify.php. If the password will be served encrypted in plain text php file would be impossible to change the password because people don't have the chance to modify the password... why do not you read the php manual before writing?
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

having a separate php file to generate a password hash would involve two things:

  1. the file manager would no longer be distributed on a single file
  2. gen_password.php (for example) should not be accessible to the public

and I cannot figure the security hole if someone has access to gen_password.php for example...

<!-- gh-comment-id:430725588 --> @alecos71 commented on GitHub (Oct 17, 2018): having a separate php file to generate a password hash would involve two things: 1) the file manager would no longer be distributed on a single file 2) gen_password.php (for example) should not be accessible to the public and I cannot figure the security hole if someone has access to gen_password.php for example...
Author
Owner

@hpolonkoev commented on GitHub (Oct 17, 2018):

@alecos71
I think you misunderstood the point of @saderror256. He don't "like" the fact that we can see the passwords plain text in php file. That all.
What I tried to suggest is to hash the password somewhere else, copy the hash and replace it in the auth_users array.

I didn't asked to update library (git repository). Just a solution for @saderror256 case. Maybe not the best one.

<!-- gh-comment-id:430728254 --> @hpolonkoev commented on GitHub (Oct 17, 2018): @alecos71 I think you misunderstood the point of @saderror256. He don't "like" the fact that we can see the passwords plain text in php file. That all. What I tried to suggest is to hash the password somewhere else, copy the hash and replace it in the auth_users array. I didn't asked to update library (git repository). Just a solution for @saderror256 case. Maybe not the best one.
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

Sorry, for a mistake I got the your quoted reply of @saderror256, so the suggestion to have a separate file to storing the password hash is yours...

for making your propose will be necessary provide a separate php file to store the password, I think that is not the better way since would be dangerous in my humble opinion. At the moment, if the server would be down no file would be readable so the risk to read the source php file is very low, should crash the php engine for reading the php file source...

<!-- gh-comment-id:430734328 --> @alecos71 commented on GitHub (Oct 17, 2018): Sorry, for a mistake I got the your quoted reply of @saderror256, so the suggestion to have a separate file to storing the password hash is yours... for making your propose will be necessary provide a separate php file to store the password, I think that is not the better way since would be dangerous in my humble opinion. At the moment, if the server would be down no file would be readable so the risk to read the source php file is very low, should crash the php engine for reading the php file source...
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

@saderror256 sorry for the mistake...

<!-- gh-comment-id:430734976 --> @alecos71 commented on GitHub (Oct 17, 2018): @saderror256 sorry for the mistake...
Author
Owner

@hpolonkoev commented on GitHub (Oct 17, 2018):

@alecos71
No worries. I think we are all here trying to find some kind of solutions and make the library better.

I totally agree with you. The purpose of manager to be a single file. The solution I suggested was just for @saderror256, not really for the library it self.

<!-- gh-comment-id:430740141 --> @hpolonkoev commented on GitHub (Oct 17, 2018): @alecos71 No worries. I think we are all here trying to find some kind of solutions and make the library better. I totally agree with you. The purpose of manager to be a single file. The solution I suggested was just for @saderror256, not really for the library it self.
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

@saderror256 something like this?

gen_password.php

<form action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post">
<span>Password:&nbsp;&nbsp;<input type="password" name="pass" id="pass">&nbsp;&nbsp;<input readonly="readonly" type="text" style="width: 440px" name="pass_cripted" id="pass_cripted" value="<?php echo isset($_POST['pass']) && !empty($_POST['pass']) ? password_hash($_POST['pass'], PASSWORD_DEFAULT) : ''; ?>"></span>
<span><input type="submit" name="submit" value="Generate"></span><br />
</form>

test.php

<?php
// the hash must enclosed in single quotes otherwise won't work
$passw_hash = '$2y$10$7uHdVGyjJjIlTkjq9vemD.hJIaLcPlc0vILKUsgRF.VwT7S6Mx9eS';
if (password_verify($_POST['password'], $passw_hash)) {
  echo "password is valid";
} else {
  echo "password is not valid";
}
?>

Not tested write on fly... as you can see for password_verify is necessary to pass the password in clear... this is the limit...

@hpolonkoev thanks!

<!-- gh-comment-id:430741935 --> @alecos71 commented on GitHub (Oct 17, 2018): @saderror256 something like this? gen_password.php ``` <form action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post"> <span>Password:&nbsp;&nbsp;<input type="password" name="pass" id="pass">&nbsp;&nbsp;<input readonly="readonly" type="text" style="width: 440px" name="pass_cripted" id="pass_cripted" value="<?php echo isset($_POST['pass']) && !empty($_POST['pass']) ? password_hash($_POST['pass'], PASSWORD_DEFAULT) : ''; ?>"></span> <span><input type="submit" name="submit" value="Generate"></span><br /> </form> ``` test.php ``` <?php // the hash must enclosed in single quotes otherwise won't work $passw_hash = '$2y$10$7uHdVGyjJjIlTkjq9vemD.hJIaLcPlc0vILKUsgRF.VwT7S6Mx9eS'; if (password_verify($_POST['password'], $passw_hash)) { echo "password is valid"; } else { echo "password is not valid"; } ?> ``` Not tested write on fly... as you can see for password_verify is necessary to pass the password in clear... this is the limit... @hpolonkoev thanks!
Author
Owner

@hpolonkoev commented on GitHub (Oct 17, 2018):

// Users: array('Username' => 'Password', 'Username2' => 'Password2', ...)
// user should generate the hash himself using the password_hash('your password', PASSWORD_DEFAULT) and add the hash into array. 
// 'login' => hashed password
$auth_users = array(
    'admin' => '$2y$10$H2ZfYowJmyVy9BjkNiWG7ubvNylwcFhvGYhUEVI7WbrB/a8Mh0bR.', // admin
    'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // 12345
);

If the user can't generate the hash himself, there are plenty of online hash generators that can be used.
The inconvenience : the user has to remember the passwords, otherwise he has to reset them.

<!-- gh-comment-id:430756383 --> @hpolonkoev commented on GitHub (Oct 17, 2018): ``` // Users: array('Username' => 'Password', 'Username2' => 'Password2', ...) // user should generate the hash himself using the password_hash('your password', PASSWORD_DEFAULT) and add the hash into array. // 'login' => hashed password $auth_users = array( 'admin' => '$2y$10$H2ZfYowJmyVy9BjkNiWG7ubvNylwcFhvGYhUEVI7WbrB/a8Mh0bR.', // admin 'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // 12345 ); ``` If the user can't generate the hash himself, there are plenty of online hash generators that can be used. The inconvenience : the user has to remember the passwords, otherwise he has to reset them.
Author
Owner

@alecos71 commented on GitHub (Oct 17, 2018):

// Users: array('Username' => 'Password', 'Username2' => 'Password2', ...)
// user should generate the hash himself using the password_hash('your password', PASSWORD_DEFAULT) and add the hash into array. 
// 'login' => hashed password
$auth_users = array(
    'admin' => '$2y$10$H2ZfYowJmyVy9BjkNiWG7ubvNylwcFhvGYhUEVI7WbrB/a8Mh0bR.', // admin
    'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // 12345
);

If the user can't generate the hash himself, there are plenty of online hash generators that can be used.
The inconvenience : the user has to remember the passwords, otherwise he has to reset them.

Nice solution, but online there are only MD5, Crypt, the only website that supports password_hash (for using with password_verify) is http://www.passwordtool.hu/php5-password-hash-generator

Thanks.

<!-- gh-comment-id:430783043 --> @alecos71 commented on GitHub (Oct 17, 2018): > > > ``` > // Users: array('Username' => 'Password', 'Username2' => 'Password2', ...) > // user should generate the hash himself using the password_hash('your password', PASSWORD_DEFAULT) and add the hash into array. > // 'login' => hashed password > $auth_users = array( > 'admin' => '$2y$10$H2ZfYowJmyVy9BjkNiWG7ubvNylwcFhvGYhUEVI7WbrB/a8Mh0bR.', // admin > 'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // 12345 > ); > ``` > > If the user can't generate the hash himself, there are plenty of online hash generators that can be used. > The inconvenience : the user has to remember the passwords, otherwise he has to reset them. Nice solution, but online there are only MD5, Crypt, the only website that supports password_hash (for using with password_verify) is http://www.passwordtool.hu/php5-password-hash-generator Thanks.
Author
Owner

@alecos71 commented on GitHub (Oct 18, 2018):

just to do something I released this little script of mine:

https://github.com/alecos71/password-hash-generator

<!-- gh-comment-id:430966676 --> @alecos71 commented on GitHub (Oct 18, 2018): just to do something I released this little script of mine: https://github.com/alecos71/password-hash-generator
Author
Owner

@alecos71 commented on GitHub (Oct 18, 2018):

https://github.com/prasathmani/tinyfilemanager/pull/73

Added my password generator...

<!-- gh-comment-id:431083582 --> @alecos71 commented on GitHub (Oct 18, 2018): https://github.com/prasathmani/tinyfilemanager/pull/73 Added my password generator...
Author
Owner

@ghost commented on GitHub (Oct 20, 2018):

This looks exactly what would be good, an idea since you want to keep everything in one file is to use comments with random strings (no reason, just spam on the keyboard) and then later, we can have a script at the start which reads the actual program for the first random string (this string is used to tell the program that the password and user has not been setup), then we show the setup, once the user picks a password and stuff, encrypt it and replace the comment with something like this

$auth_users = array(
    'admin' => $ww$ww$OurencryptedusernameKIjdaiousdj543DADSg$, // From account setup
    'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // Also account setup
);

This way, it still stays in one file 👍

<!-- gh-comment-id:431537619 --> @ghost commented on GitHub (Oct 20, 2018): This looks exactly what would be good, an idea since you want to keep everything in one file is to use comments with random strings (no reason, just spam on the keyboard) and then later, we can have a script at the start which reads the actual program for the first random string (this string is used to tell the program that the password and user has not been setup), then we show the setup, once the user picks a password and stuff, encrypt it and replace the comment with something like this ``` $auth_users = array( 'admin' => $ww$ww$OurencryptedusernameKIjdaiousdj543DADSg$, // From account setup 'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // Also account setup ); ``` This way, it still stays in one file :+1:
Author
Owner

@alecos71 commented on GitHub (Oct 20, 2018):

What about my implementation? It does exactly what request, read and test in localhost my changes...

https://github.com/prasathmani/tinyfilemanager/pull/73/files#diff-f382ee4072618879bd7063ce8e903db7

It works so that:

if a password is empty, fm will show the password generator, then copy and past into array:

$auth_users = array(
    // insert the generated password for 'admin' inside the single quotes, save everything and reload the page
    'admin' => '',
    // insert the generated password for 'user' inside the single quotes, save everything and reload the page
    'user' => ''
);
$auth_users = array(
    // insert the generated password for 'admin' inside the single quotes, save everything and reload the page
    'admin' => '$2y$10$H2ZfYowJmyVy9BjkNiWG7ubvNylwcFhvGYhUEVI7WbrB/a8Mh0bR.', // admin
    // insert the generated password for 'user' inside the single quotes, save everything and reload the page
    'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // 12345
);

save filemanager php file and reload the page, since now the passwords are not empty fm will show the login, it works even if $use_auth = false;

The password generator is inside the one php filemanager so there is no necessity to have another file...

immagine

<!-- gh-comment-id:431562276 --> @alecos71 commented on GitHub (Oct 20, 2018): What about my implementation? It does exactly what request, read and test in localhost my changes... https://github.com/prasathmani/tinyfilemanager/pull/73/files#diff-f382ee4072618879bd7063ce8e903db7 It works so that: if a password is empty, fm will show the password generator, then copy and past into array: ``` $auth_users = array( // insert the generated password for 'admin' inside the single quotes, save everything and reload the page 'admin' => '', // insert the generated password for 'user' inside the single quotes, save everything and reload the page 'user' => '' ); ``` ``` $auth_users = array( // insert the generated password for 'admin' inside the single quotes, save everything and reload the page 'admin' => '$2y$10$H2ZfYowJmyVy9BjkNiWG7ubvNylwcFhvGYhUEVI7WbrB/a8Mh0bR.', // admin // insert the generated password for 'user' inside the single quotes, save everything and reload the page 'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // 12345 ); ``` save filemanager php file and reload the page, since now the passwords are not empty fm will show the login, it works even if `$use_auth = false;` The password generator is inside the one php filemanager so there is no necessity to have another file... ![immagine](https://user-images.githubusercontent.com/9997666/47254361-bf3a8600-d461-11e8-92dd-c33d04e515dd.png)
Author
Owner

@alecos71 commented on GitHub (Oct 20, 2018):

This looks exactly what would be good, an idea since you want to keep everything in one file is to use comments with random strings (no reason, just spam on the keyboard) and then later, we can have a script at the start which reads the actual program for the first random string (this string is used to tell the program that the password and user has not been setup), then we show the setup, once the user picks a password and stuff, encrypt it and replace the comment with something like this

$auth_users = array(
    'admin' => $ww$ww$OurencryptedusernameKIjdaiousdj543DADSg$, // From account setup
    'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // Also account setup
);

This way, it still stays in one file 👍

This is very dangerous since filemanager should not able to write into its file because if hacker can access to fm and generate its passwords will be the end of game... instead should work as my propose...

<!-- gh-comment-id:431563544 --> @alecos71 commented on GitHub (Oct 20, 2018): > > > This looks exactly what would be good, an idea since you want to keep everything in one file is to use comments with random strings (no reason, just spam on the keyboard) and then later, we can have a script at the start which reads the actual program for the first random string (this string is used to tell the program that the password and user has not been setup), then we show the setup, once the user picks a password and stuff, encrypt it and replace the comment with something like this > > ``` > $auth_users = array( > 'admin' => $ww$ww$OurencryptedusernameKIjdaiousdj543DADSg$, // From account setup > 'user' => '$2y$10$3nkHPMgWAwhfsjK3D0vIOO9QWlClVW3gGuNQwO19opZFkg0tv50pa', // Also account setup > ); > ``` > > This way, it still stays in one file 👍 This is very dangerous since filemanager should not able to write into its file because if hacker can access to fm and generate its passwords will be the end of game... instead should work as my propose...
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/tinyfilemanager#54
No description provided.