Bug 629882 – <net-analyzer/zabbix-{3.0.30,4.0.18,4.4.6}: root privilege escalation via config file manipulation

• Status: RESOLVED FIXED
• Product: Gentoo Security

[Description Michael Orlitzky 2017-09-04 14:40:26 UTC]

The zabbix ebuilds give ownership of their config files to the "zabbix" users:

  if use server; then
      ...
      fowners zabbix:zabbix /etc/zabbix/zabbix_server.conf

  ...

  fowners zabbix:zabbix \
		/etc/zabbix
  ...

This is probably safe when starting the daemon via the init script because start-stop-daemon drops privileges immediately. But if you try to run the zabbix server from the command-line as root, it's exploitable by the "zabbix" user to gain root. This is due to a few sensitive settings being stored in the zabbix_server.conf file:

  
  # AllowRoot=0
  # User=zabbix
  # AlertScriptsPath=${datadir}/zabbix/alertscripts

The "zabbix" user can write to that configuration file, so he can set,

  AllowRoot=1
  User=root
  AlertScriptsPath=/path/to/goodies

The next time the zabbix server is started from the command-line, it will run as root and execute some scripts chosen by the "zabbix" user.

Untested possible fix: make everything root:zabbix and mode 750 or 640 instead.


Unrelated aesthetic improvement: if you make use of the OpenRC variables like command, command_args, command_user, etc., then your start() and stop() functions can be completely eliminated in the init scripts.

[Comment 1 Michael Orlitzky 2019-09-14 16:05:53 UTC]

Two-year ping =)

[Comment 2 Thomas Deutschmann (RETIRED) 2019-10-26 20:56:02 UTC]

CC'ing treecleaners. Serious, root priv escalation and still no reaction after 2 years...

[Comment 3 Larry the Git Cow 2019-10-27 07:27:47 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=f8e5c171a235773e625d8db6811674ff7dfe15c7

commit f8e5c171a235773e625d8db6811674ff7dfe15c7
Author:     Michał Górny <[e-mail]>
AuthorDate: 2019-10-27 07:27:20 +0000
Commit:     Michał Górny <[e-mail]>
CommitDate: 2019-10-27 07:27:20 +0000

    package.mask: Last rite net-analyzer/zabbix
    
    Bug: https://bugs.gentoo.org/629882
    Signed-off-by: Michał Górny <[e-mail]>

 profiles/package.mask | 6 ++++++
 1 file changed, 6 insertions(+)

[Comment 4 James Le Cuirot 2019-10-27 09:57:28 UTC]

I don't use Zabbix any more as my work is now more dev-focused and I never ran it on Gentoo but it would be a shame to see this great software go. This is a trivial fix and Gentoo is not far behind upstream.

I maintained a Chef cookbook for Zabbix and it installed zabbix_server.conf as root:root with 640. As long as the file has User=zabbix, I don't know why you'd need to drop privileges before it starts.

I realise there's a pile of other bugs too but at a glance, they don't look too bad. No, I'm not going to fix them though, sorry. :(

[Comment 5 Mikle Kolyada (RETIRED) 2019-10-27 11:02:47 UTC]

(In reply to James Le Cuirot from comment #4)
> I don't use Zabbix any more as my work is now more dev-focused and I never
> ran it on Gentoo but it would be a shame to see this great software go. This
> is a trivial fix and Gentoo is not far behind upstream.
> 
> I maintained a Chef cookbook for Zabbix and it installed zabbix_server.conf
> as root:root with 640. As long as the file has User=zabbix, I don't know why
> you'd need to drop privileges before it starts.
> 
> I realise there's a pile of other bugs too but at a glance, they don't look
> too bad. No, I'm not going to fix them though, sorry. :(

I agree that it is a shame for production users, maybe we should rather drop it to the maintainer-needed

[Comment 6 Mark Nowiasz 2019-10-27 13:03:51 UTC]

This is one of the wideley used enterprise monitoring systems (apart from the companies I've worked I'm using this for monitoring half a dozen servers) and removing this would make Gentoo a laughing stock - you could as well remove nagios/icinga or apache2.

[Comment 7 Patrick Lauer 2019-10-27 13:07:06 UTC]

commit 46cfee722d2957dd15a285a4d81373bdad834c36
Author: Patrick Lauer <[e-mail]>
Date:   Sun Oct 27 12:57:42 2019 +0000

    net-analyzer/zabbix: Tweak permissions

    Fix #629882

[Comment 8 Vadim A. Misbakh-Soloviov (mva) 2019-11-07 10:09:50 UTC]

I'm using zabbix, but I can't sign up as active maintainer, although, I'd be happy to co-maintain it with somebody else.

BTW, @mgorny, as I see, Patrick already fixed the issue, so can we talk about unmasking and un-lastriting zabbix now?

[Comment 9 Tomáš Mózes 2019-11-07 10:26:29 UTC]

(In reply to Mark Nowiasz from comment #6)
> This is one of the wideley used enterprise monitoring systems (apart from
> the companies I've worked I'm using this for monitoring half a dozen
> servers) and removing this would make Gentoo a laughing stock - you could as
> well remove nagios/icinga or apache2.

Please don't put everything into one big bag. Nobody is going to remove apache/nagios, they are properly maintained.

If the maintainer doesn't care of a security bug for 2 years, maybe he's not interested in the package as a whole in which case it's better to remove it than to leave in Gentoo.

If anyone is an active Zabbix user, it's better to join as maintainer than to wait for it to rot and die and be removed.

[Comment 10 Andrey Volkov 2019-12-17 20:09:09 UTC]

Commit 46cfee722d2957dd15a285a4d81373bdad834c36 is not enough

"zabbix" user has permission to replace/delete configuration files as /etc/zabbix owner. 

Please change ownership for /etc/zabbix also.

[Comment 11 Larry the Git Cow 2020-02-19 07:46:53 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=0f27532b104c6463ee4a8897148afe5de949a333

commit 0f27532b104c6463ee4a8897148afe5de949a333
Author:     Michał Górny <[e-mail]>
AuthorDate: 2020-02-19 07:46:05 +0000
Commit:     Michał Górny <[e-mail]>
CommitDate: 2020-02-19 07:46:48 +0000

    package.mask: Mask net-analyzer/zabbix for vulnerabilities
    
    Bug: https://bugs.gentoo.org/629882
    Bug: https://bugs.gentoo.org/629884
    Signed-off-by: Michał Górny <[e-mail]>

 profiles/package.mask | 7 +++++++
 1 file changed, 7 insertions(+)

[Comment 12 Felix Tiede 2020-02-20 07:36:49 UTC]

ebuild for net-analyzer/zabbix-4.4.5 is seemingly fixed in the tree, as configuration files are created as root:zabbix with 0640 permissions by default, as tested on numerous systems.

[Comment 13 Michael Orlitzky 2020-02-20 12:26:36 UTC]

(In reply to Felix Tiede from comment #12)
> ebuild for net-analyzer/zabbix-4.4.5 is seemingly fixed in the tree, as
> configuration files are created as root:zabbix with 0640 permissions by
> default, as tested on numerous systems.

The files are OK, it's the parent directory that's a problem now. The ebuild does,

  fowners zabbix:zabbix \
    /etc/zabbix \
    ...

and afterwards the permissions on the files don't matter, because zabbix can simply delete and recreate any of them.

[Comment 14 Vadim A. Misbakh-Soloviov (mva) 2020-02-27 20:04:04 UTC]

By the way, I have one question about the bug's essence:

Why on the earth wuould you run zabbix-as-root directly from commandline, and not as zabbix-as-zabbix-user from the service?


I don't think it is REAL priv escalation issue because, well, you can also run /home/eviluser/trojan being logged as root too.

[Comment 15 Thomas Deutschmann (RETIRED) 2020-02-27 20:10:38 UTC]

See comment #0: The service is getting started through s-s-d for example which usually runs as root. Daemon will drop privileges later. However, because configuration which tells daemon to drop privileges is writable for service itself, a compromised service could drop instruction to drop privileges allowing a compromised service to escalate privileges when administrator will restart service next time.

[Comment 16 Larry the Git Cow 2020-02-28 15:02:15 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=9dd83ba9636be855abf97ac682cd55be731f0ce2

commit 9dd83ba9636be855abf97ac682cd55be731f0ce2
Author:     Miroslav Šulc <[e-mail]>
AuthorDate: 2020-02-28 15:01:10 +0000
Commit:     Miroslav Šulc <[e-mail]>
CommitDate: 2020-02-28 15:02:00 +0000

    net-analyzer/zabbix: bumps + security fixes + rewritten + removed obsolete
    
    1) many changes and improvements
    2) config directory and files are not writeable by zabbix
    3) creation of pid file disabled in zabbix, using s-s-d instead
    
    Bug: https://bugs.gentoo.org/629882
    Bug: https://bugs.gentoo.org/709926
    Bug: https://bugs.gentoo.org/629884
    Closes: https://bugs.gentoo.org/665960
    Closes: https://bugs.gentoo.org/670652
    Package-Manager: Portage-2.3.89, Repoman-2.3.20
    Signed-off-by: Miroslav Šulc <[e-mail]>

 net-analyzer/zabbix/Manifest                       |  10 +-
 net-analyzer/zabbix/files/2.2/init.d/zabbix-agentd |  28 -
 net-analyzer/zabbix/files/2.2/init.d/zabbix-proxy  |  27 -
 net-analyzer/zabbix/files/2.2/init.d/zabbix-server |  26 -
 .../zabbix/files/2.2/patches/zbx7479.patch         |  83 ---
 .../zabbix/files/2.2/patches/zbx8151.patch         |  53 --
 net-analyzer/zabbix/files/2.2/zabbix_agent.conf    |  81 ---
 net-analyzer/zabbix/files/2.2/zabbix_agentd.conf   | 278 ---------
 net-analyzer/zabbix/files/2.2/zabbix_proxy.conf    | 519 ----------------
 net-analyzer/zabbix/files/2.2/zabbix_server.conf   | 546 -----------------
 net-analyzer/zabbix/files/3.0/init.d/zabbix-agentd |  28 -
 net-analyzer/zabbix/files/3.0/init.d/zabbix-proxy  |  27 -
 net-analyzer/zabbix/files/3.0/init.d/zabbix-server |  26 -
 net-analyzer/zabbix/files/3.0/zabbix_agent.conf    |  81 ---
 net-analyzer/zabbix/files/3.0/zabbix_agentd.conf   | 390 ------------
 net-analyzer/zabbix/files/3.0/zabbix_proxy.conf    | 674 ---------------------
 net-analyzer/zabbix/files/3.0/zabbix_server.conf   | 635 -------------------
 .../zabbix/files/zabbix-3.0.30-mysql8.patch        |  17 +
 .../zabbix-3.0.30-security-disable-PidFile.patch   |  49 ++
 ...fix.patch => zabbix-4.0.18-modulepathfix.patch} |   0
 .../zabbix-4.0.18-security-disable-PidFile.patch   |  49 ++
 net-analyzer/zabbix/files/zabbix-agentd.init       |  20 +
 net-analyzer/zabbix/files/zabbix-agentd.service    |  10 +-
 .../zabbix-jmx-proxy => zabbix-jmx-proxy.conf}     |   0
 .../zabbix-jmx-proxy => zabbix-jmx-proxy.init}     |   0
 net-analyzer/zabbix/files/zabbix-proxy.init        |  20 +
 net-analyzer/zabbix/files/zabbix-proxy.service     |   8 +-
 net-analyzer/zabbix/files/zabbix-server.init       |  19 +
 net-analyzer/zabbix/files/zabbix-server.service    |  11 +-
 net-analyzer/zabbix/zabbix-2.2.16-r1.ebuild        | 340 -----------
 net-analyzer/zabbix/zabbix-3.0.28.ebuild           | 330 ----------
 .../{zabbix-3.4.15.ebuild => zabbix-3.0.30.ebuild} | 204 ++++---
 net-analyzer/zabbix/zabbix-4.0.13.ebuild           | 332 ----------
 .../{zabbix-4.2.7.ebuild => zabbix-4.0.18.ebuild}  | 207 ++++---
 net-analyzer/zabbix/zabbix-4.4.0-r1.ebuild         | 333 ----------
 .../{zabbix-4.4.5.ebuild => zabbix-4.4.6.ebuild}   | 204 ++++---
 36 files changed, 523 insertions(+), 5142 deletions(-)

[Comment 17 Miroslav Šulc 2020-02-28 15:06:09 UTC]

please review the fix whether it is sufficient. keeping masked for now.

[Comment 18 Michael Orlitzky 2020-03-06 14:09:45 UTC]

It looks OK to me now, thanks for fixing this. Here are some other non-critical suggestions:

  * The use of checkpath in the OpenRC script should not be needed. The
    directory /etc/zabbix and /etc/zabbix/*.conf are created with the
    correct permissions and ownership by the ebuild, so what is the
    checkpath supposed to do? (It's harmless, just redundant.)

  * Now that the daemon is run in the foreground using both OpenRC and
    systemd, I don't think we need the "tmpfiles" entry that creates
    /run/zabbix. That directory only existed to hold the PID file when
    the daemon itself created the PID file.

  * At the top of src_install, we do

	local dirs=(
		/etc/zabbix
		/var/lib/zabbix
		/var/lib/zabbix/home
		/var/lib/zabbix/scripts
		/var/lib/zabbix/alertscripts
		/var/lib/zabbix/externalscripts
		/var/log/zabbix
	)

	for dir in "${dirs[@]}"; do
		dodir "${dir}"
		keepdir "${dir}"
	done

    But then later (at the end of src_install) we go back and mess
    with those directories, e.g.

	fperms 0750 \
		/etc/zabbix \
		/var/lib/zabbix \
		/var/lib/zabbix/home \
		/var/lib/zabbix/scripts \
		/var/lib/zabbix/alertscripts \
		/var/lib/zabbix/externalscripts \
		/var/log/zabbix

    Why not just use diropts to set the permissions/ownership when you
    create them? (Just be careful not to make /etc/zabbix owned by zabbix!)

[Comment 19 Francesco Riosa 2020-03-18 10:02:25 UTC]

(In reply to Michael Orlitzky from comment #18)
>   * The use of checkpath in the OpenRC script should not be needed. The
>     directory /etc/zabbix and /etc/zabbix/*.conf are created with the
>     correct permissions and ownership by the ebuild, so what is the
>     checkpath supposed to do? (It's harmless, just redundant.)
> 
I disagree, the ebuild is active only at install time, having a check every time the service is started is a good thing.
I don't see creating the directory from the ebuild as bad, but
if anything is redundant is the ebuild that need to be pruned, not the init scripts

[Comment 20 Tomáš Mózes 2020-03-18 10:51:04 UTC]

(In reply to Michael Orlitzky from comment #18)
> It looks OK to me now, thanks for fixing this. Here are some other
> non-critical suggestions:

Can the mask please be removed then? My servers with agents installed are triggering the monitoring about packages being in weird state (installed, but masked). 

Thank you!

[Comment 21 Michael Orlitzky 2020-03-18 13:27:22 UTC]

(In reply to Francesco Riosa from comment #19)
> 
> I disagree, the ebuild is active only at install time, having a check every
> time the service is started is a good thing.
> I don't see creating the directory from the ebuild as bad, but
> if anything is redundant is the ebuild that need to be pruned, not the init
> scripts

Checkpath is itself insecure. In general, creating the wrong permissions and then trying to "fix" them automatically with chown/chmod is exploitable. Checkpath calls stat() before running the vulnerable operation, but that just turns the easy exploit into a less-easy race condition. And you get a chance to win the race every time the service starts, if checkpath is called in the init script.

In this case it's fine because checkpath is setting read-only permissions for the non-root user. But unless we expect everyone to be an expert on these security issues and to have read every old openrc bug that was closed with a half-assed fix, it's better to just avoid them. The permissions on a root-owned directory will never change unless root changes them, and in that case we probably shouldn't contradict the administrator anyway (if he wants to add a g+w ACL, for example).

[Comment 22 Miroslav Šulc 2020-03-19 17:34:03 UTC]

the reason i added checkpath was that if not added, already installed instance of zabbix was not updated with the new ownership from the ebuilds which i considered insecure. on the other hand, i admit that this prevents admins from changing the ownership to their will.

shall i do some other changes to the ebuild so that if can be unmasked or it can be unmasked as it is?

[Comment 23 Michael Orlitzky 2020-03-19 17:44:30 UTC]

(In reply to Miroslav Šulc from comment #22)
> the reason i added checkpath was that if not added, already installed
> instance of zabbix was not updated with the new ownership from the ebuilds
> which i considered insecure. on the other hand, i admit that this prevents
> admins from changing the ownership to their will.

It's not a big deal. I only mentioned it because I had to read through everything carefully anyway, and making a note of the potential improvements I saw seemed like the nice thing to do.

> shall i do some other changes to the ebuild so that if can be unmasked or it
> can be unmasked as it is?

The QA flag got waved on this one and I'm not on the QA team, but as the original reporter maybe I get a vote. The issue is solved. I think you can go ahead and unmask it.

[Comment 24 Larry the Git Cow 2020-03-20 10:10:46 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=c13d1a00d3372475df99db6c23a90ad0294a3252

commit c13d1a00d3372475df99db6c23a90ad0294a3252
Author:     Miroslav Šulc <[e-mail]>
AuthorDate: 2020-03-20 10:08:47 +0000
Commit:     Miroslav Šulc <[e-mail]>
CommitDate: 2020-03-20 10:09:02 +0000

    package.mask: unmasked net-analyzer/zabbix
    
    Bug: https://bugs.gentoo.org/629882
    Bug: https://bugs.gentoo.org/629884
    Bug: https://bugs.gentoo.org/709926
    Signed-off-by: Miroslav Šulc <[e-mail]>

 profiles/package.mask | 7 -------
 1 file changed, 7 deletions(-)

[Comment 25 Thomas Deutschmann (RETIRED) 2020-03-20 18:39:45 UTC]

@ Miroslav: Did you de-stabilize zabbix on purpose? If not, please call for stabilization!

[Comment 26 Miroslav Šulc 2020-03-21 06:34:06 UTC]

(In reply to Thomas Deutschmann from comment #25)
> @ Miroslav: Did you de-stabilize zabbix on purpose? If not, please call for
> stabilization!

yes, it was on purpose as i did large rewrite of the ebuilds. anyway, i have it already running for over 20 days on my servers and all seems fine so requesting stabilization now.

[Comment 27 Agostino Sarubbo 2020-03-21 16:47:27 UTC]

amd64 stable

[Comment 28 Agostino Sarubbo 2020-03-22 14:07:50 UTC]

x86 stable

[Comment 29 Sam James 2020-04-02 08:51:24 UTC]

Tree looks clean.

[Comment 30 NATTkA bot 2020-04-12 19:31:21 UTC]

Resetting sanity check; package list is empty or all packages are done.

[Comment 31 NATTkA bot 2020-05-14 06:57:18 UTC]

Unable to check for sanity:

> no match for package: =net-analyzer/zabbix-4.0.18

[Comment 32 NATTkA bot 2020-06-04 07:53:18 UTC]

Unable to check for sanity:

> no match for package: =net-analyzer/zabbix-3.0.30

[Comment 33 GLSAMaker/CVETool Bot 2021-01-21 19:18:48 UTC]

This issue was resolved and addressed in
 GLSA 202101-11 at https://security.gentoo.org/glsa/202101-11
by GLSA coordinator Aaron Bauman (b-man).