Creating a root capable exploit with change password helper

[iliajie]

Hello,

I am on the team of developers of Virtualmin and would like to inform you, that such suggestion:

roundcubemail/plugins/password/helpers/chgvirtualminpasswd.c

Lines 6 to 13 in 8547ccb

	#define CMD "/usr/sbin/virtualmin"
	/* INSTALLING:
	gcc -o chgvirtualminpasswd chgvirtualminpasswd.c
	chown root.apache chgvirtualminpasswd
	strip chgvirtualminpasswd
	chmod 4550 chgvirtualminpasswd
	*/

.. in combination with virtualmin.php:

exec("$curdir/chgvirtualminpasswd modify-user --domain $domain --user $username --pass $newpass", $output, $returnvalue);

.. is equivalent to creating root capable exploit, because virtualmin command is meant to be run by root user only.

For example a regular user could modify virtualmin.php file and/or run as un-privileged user:

doms=`./chgvirtualminpasswd list-domains`
for dom in $doms; do
  ./chgvirtualminpasswd delete-domain --domain $dom
done

I suggest to hardcode the command chgvirtualminpasswd modify-user --domain $domain --user $username --pass $newpass from PHP script into C script and later let binary receive a domain, a username and a password as parameters from PHP script, like:

exec("$curdir/chgvirtualminpasswd $domain $username $newpass", $output, $returnvalue);

By the way main() {} on chgvirtualminpasswd.c should be int main() {}, otherwise recent versions of compilers will fail.

[iliajie]

.. even though with mentioned implementation above it would still be possible to change other users passwords. Not acceptable as well. The right way would be is to check/hardcode the current unix user and make sure that it passed to --user param in non-arbitrary way.

[alecpl]

Would you like to provide a pull request?

[iliajie]

Would you like to provide a pull request?

Yes, sure.

The problem is that after I started looking into this issue closely I discovered that using virtualmin modify-user is not going to work reliably even if we test current user with getenv("USER") as there are users with /dev/null and /bin/false shells. The right solution would be is to use virtualmin change-password command, which starting with Virtualmin 6.17+ version will accept three arguments - user, oldpass, newpass.

This PR will provide with chgvirtualminpasswd command, which you later should use on your PHP script virtualmin.php as simple as:

exec("$curdir/chgvirtualminpasswd $username $oldpass $newpass", $output_domain, $returnvalue);

---

.. possible stdouts with 256 exit error code are:

Not a Virtualmin user    # when supplied user is incorrect
Wrong password           # when old password is incorrect

.. possible stdout with 0 exit success code:

Password changed for username in domain

[alecpl]

So, this is going to work with most recent Virtualmin version only?

What do you think about rewriting the driver to use Remote API? That could be a better solution for many reasons. Would that work? Would be the version limitation be the same?

[iliajie]

So, this is going to work with most recent Virtualmin version only?

Yes, we encourage all of our users to install latest updates of Virtualmin (with various success) 🙂

What do you think about rewriting the driver to use Remote API?

Do you mean you as a developer being able to do the following but without using authentication:

<?php
$rs = shell_exec("wget -O - --quiet --http-user=root --http-passwd=pass 'https://h.s1.com:10000/virtual-server/remote.cgi?program=change-password&user=user1&pass=123&newpass=12345'");

If so, I don't think we would do it. From our documentation:

Because the remote API is part of the Virtualmin web interface, it is password-protected using the same authentication system as you would normally use to login via a web browser. Only the master administrator can access the remote API though, as it can be used to perform almost any action in Virtualmin, and thus would be unsafe for server owners or resellers to use.

@jcameron Jamie, do you think opening change-password to external world completely without authentication would be safe or not, considering brute forcing old password wouldn't be easily possible due to a significant delay?

@alecpl At the same time, I don't understand what is the problem in changing a password using patched C script? It's common practice for a user to enter an old password to change it, as only master administrator have an ability to set new passwords (without providing old).

Would be the version limitation be the same?

Yes, currently we don't have an ability to change user passwords with remote API without authentication.

Do not worry about users not installing upgrades - Virtualmin is kind of software that must be updated on regular basis and we have repos setup on user's server to do that as regular package update procedure.

The most concern should be about giving regular user root capable permissions. I would suggest reviewing services for familiar issue aside from already patched Virtualmin driver.

[alecpl]

Do you mean you as a developer being able to do the following but without using authentication:

<?php
$rs = shell_exec("wget -O - --quiet --http-user=root --http-passwd=pass 'https://h.s1.com:10000/virtual-server/remote.cgi?program=change-password&user=user1&pass=123&newpass=12345'");

Yes, but wrote it in PHP using Guzzle http client.

If so, I don't think we would do it. From our documentation:

Because the remote API is part of the Virtualmin web interface, it is password-protected using the same authentication system as you would normally use to login via a web browser. Only the master administrator can access the remote API though, as it can be used to perform almost any action in Virtualmin, and thus would be unsafe for server owners or resellers to use.

@jcameron Jamie, do you think opening change-password to external world completely without authentication would be safe or not, considering brute forcing old password wouldn't be easily possible due to a significant delay?

There's no need for unauthenticated access. Either store admin password in config file, or better allow the user itself to authenticate to the password-change end-point.

@alecpl At the same time, I don't understand what is the problem in changing a password using patched C script? It's common practice for a user to enter an old password to change it, as only master administrator have an ability to set new passwords (without providing old).

Not really a problem. But if we're looking at minimum version requirement bump, maybe we should look for some better approaches. And a remote API has some pros over the CLI solution. Many other drivers prefer http APIs over CLI commands.

[iliajie]

Not really a problem. But if we're looking at minimum version requirement bump

Considering that there is no API currently on our side (and it won't be there for a while) and you still need to update vulnerable helper command, I don't see any other better reasonable option.

or better allow the user itself to authenticate to the password-change end-point.

This is exactly what I do on the PR, as the right way is to user change-password command.

[alecpl]

or better allow the user itself to authenticate to the password-change end-point.

This is exactly what I do on the PR, as the right way is to user change-password command.

So, can we have this in the remote API? I think we can agree that would be the best option. No need to compile and setup anything. Multi-host environments.

[alecpl]

@iliarostovtsev we'd like to release 1.5-rc soon. Obviously we don't want to release a code that depends on unreleased virtualmin version. So, I have some questions to you.

1. When will Virtualmin 6.17 be available?
2. Do you plan to implement a safe password change via the remote API or there's no chance for that?

[iliajie]

Please re-read my comment above - #8007 (comment)

PR includes all the you need for a user to change the password, without jeopardizing the security.

We are not planning to make a new release at the moment neither add remote API. Perhaps later, as at the moment there is already too much of everything on our plates. Sorry.