Permalink
Browse files

Hardening password checks

  • Loading branch information...
Serghey Rodin
Serghey Rodin committed Apr 8, 2018
1 parent ffdae1d commit 3fdee2975db0c80419a0dfefff3c10a2c4de6410
Showing with 6 additions and 4 deletions.
  1. +1 −1 bin/v-check-user-password
  2. +1 −1 func/main.sh
  3. +3 −1 web/api/index.php
  4. +1 −1 web/login/index.php
@@ -82,7 +82,7 @@ if [ -z "$salt" ]; then
fi

# Generating hash
hash=$($BIN/v-generate-password-hash $method $salt <<< $password)
hash=$($BIN/v-generate-password-hash $method $salt <<< "$password")
if [[ -z "$hash" ]]; then
echo "Error: password missmatch"
echo "$date $time $user $ip failed to login" >> $VESTA/log/auth.log
@@ -273,7 +273,7 @@ is_object_value_exist() {
is_password_valid() {
if [[ "$password" =~ ^/tmp/ ]]; then
if [ -f "$password" ]; then
password=$(head -n1 $password)
password="$(head -n1 $password)"
fi
fi
}
@@ -18,13 +18,15 @@
fwrite($fp, $_POST['password']."\n");
fclose($fp);
$v_ip_addr = escapeshellarg($_SERVER["REMOTE_ADDR"]);
exec(VESTA_CMD ."v-check-user-password ".$v_user." ".$v_password." '".$v_ip_addr."'", $output, $auth_code);
exec(VESTA_CMD ."v-check-user-password ".$v_user." ".escapeshellarg($v_password)." '".$v_ip_addr."'", $output, $auth_code);

This comment has been minimized.

@StudioMaX

StudioMaX Apr 8, 2018

Contributor

How this can be exploited?
https://3v4l.org/pIdHE

This comment has been minimized.

@serghey-rodin

serghey-rodin Apr 8, 2018

Owner

I don't think that this particular line can be exploited. It's more for those who suggested us that password arg is unescaped.

unlink($v_password);
/* No hash auth for security reason
} else {
$key = '/usr/local/vesta/data/keys/' . basename($_POST['hash']);
if (file_exists($key) && is_file($key)) {
$auth_code = '0';
}
*/

This comment has been minimized.

@madeITBelgium

madeITBelgium Apr 8, 2018

Collaborator

This is a breaking change :(

This comment has been minimized.

@SS88UK

SS88UK Apr 8, 2018

Is there a reason API keys have now been disabled?

This comment has been minimized.

@serghey-rodin

serghey-rodin Apr 8, 2018

Owner

It's temporary... until we get more info about recent hacks. For now we just trying to minimize the risk.

This comment has been minimized.

@serghey-rodin

serghey-rodin Apr 8, 2018

Owner

Guys, could you please test my last commit before I started build. I think I found 100% safe way to communicate passwords between php and bash. Thanks

}
if ($auth_code != 0 ) {
@@ -44,7 +44,7 @@
fclose($fp);
// Check user & password
exec(VESTA_CMD ."v-check-user-password ".$v_user." ".$v_password." ".escapeshellarg($_SERVER['REMOTE_ADDR']), $output, $return_var);
exec(VESTA_CMD ."v-check-user-password ".$v_user." ".escapeshellarg($v_password)." ".escapeshellarg($_SERVER['REMOTE_ADDR']), $output, $return_var);
unset($output);
// Remove tmp file

0 comments on commit 3fdee29

Please sign in to comment.