The unrealircd init script gives the "unrealircd" user write access to its own PID file. This is a minor security vulnerability that can be used to kill processes on the system if the unrealircd daemon is compromised. The purpose of the PID file is to hold the PID of the running daemon, so that later it can be stopped, restarted, or otherwise signalled (many daemons reload their configurations in response to a SIGHUP). To fulfil that purpose, the contents of the PID file need to be trustworthy. If the PID file is writable by a non-root user, then he can replace its contents with the PID of a root process. Afterwards, any attempt to signal the PID contained in the PID file will instead signal a root process chosen by the non-root user (a vulnerability). The unrealircd init script does, if ! [ -d "${UNREALIRCD_PIDFILEDIR}" ]; then ebegin "Creating ${UNREALIRCD_PIDFILEDIR} for ${SVCNAME}" mkdir "${UNREALIRCD_PIDFILEDIR}" \ && chown :unrealircd "${UNREALIRCD_PIDFILEDIR}" \ && chmod 770 "${UNREALIRCD_PIDFILEDIR}" and so the "unrealircd" group can write to its PID file(s). The upstream situation with this one is a little bit weird. The daemon has the ability to create its own PID file, but will refuse to run as root, and is unable to drop privileges to a restricted user on its own. It's not clear how you're supposed to integrate that with an init system; fortunately, the daemon has the ability to run in the foreground. With OpenRC or systemd, you can therefore have the init system manage the PID file, drop privileges, and force the process into the background in a safe manner.
Created attachment 489870 [details] unrealircd.confd-r3
Created attachment 489872 [details] unrealircd.initd-r2
Created attachment 489874 [details] unrealircd-4.0.13.ebuild There was a new version of unrealircd released a few days ago, so here's a version bump for it. I've rewritten the init script to run the daemon in the foreground, and manage the PID file securely. I hit another issue with my hardened kernel: it refuses to run code from the group-writable /var/lib/unrealircd/tmp directory. I was able to fix that by making it unrealircd:root rather than root:unrealircd. There's a comment about that in the ebuild, though, so I've left it alone in the version bump.
I think it's natural to notice these things 10 seconds too late: the "SVCNAME" variable used in the init/confd file should instead be the modern "RC_SVCNAME". Both work, but now's as good a time as ever to update it.
Thanks. I should have time to test and commit the bump tomorrow if you want me to do that. If it is considered more urgent, please go ahead and I will still test it tomorrow after the fact. I have wondered before about how the daemon could, e.g., listen to TCP port requiring privileges when it would refuse to launch as root. Upstream does not consider system init scripts a very supported use case, but would probably accept patches to allow “normal” daemonization if guarded by a compiletime switch. The issue of not supporting the hardened kernel is discussed in bug #223835. I think that can and should be fixed (upstream is against this because it breaks module reloading but with Gentoo you’d probably still need to restart the ircd).
It's far from urgent. I probably should report this upstream even though I'm having a hard time wrapping my head around it. Maybe no one is using the daemon-created PID file, but since the daemon will only run as a restricted user, if anyone *did* try to use its self-created PID file, there would be the same vulnerability. The only mitigating factor is that it's really awkward to do so. If upstream really doesn't like thinking about such things, they could always eliminate the PID file entirely and just suggest that people use a more sophisticated init system like OpenRC/systemd.
Michael or Nathan, does this only affect 4.0 or is 3.2.10.7 also affected. This would depend on what we have to do with the security portion of it.
(In reply to Yury German from comment #7) > Michael or Nathan, does this only affect 4.0 or is 3.2.10.7 also affected. > This would depend on what we have to do with the security portion of it. Both versions, via the init scripts.
@mjo One question about the init scripts changes: why should UNREALIRCD_USER be removed? That change works against the ideal situation of being able to start multiple instances of unrealircd sandboxed to their own users (this probably doesn’t work, though, but hardcoding the username prevents that entirely). Is it OK if I add that back in or are you addressing some issue with that change?
(In reply to Nathan Phillip Brink (binki) from comment #9) > @mjo > > One question about the init scripts changes: why should UNREALIRCD_USER be > removed? That change works against the ideal situation of being able to > start multiple instances of unrealircd sandboxed to their own users (this > probably doesn’t work, though, but hardcoding the username prevents that > entirely). Is it OK if I add that back in or are you addressing some issue > with that change? Two reasons: 1. Changing it doesn't work. 2. Most FOO_USER variables inevitably lead to a vulnerability. The first is obvious; in the ebuild you take great care to set everything up just right for the "unrealircd" user. If the end-user tries to change UNREALIRCD_USER, all of the permissions on his system are wrong for the daemon to run. The second is more subtle (and totally theoretical at this point). When you have a daemon that creates files (like logs) and its runtime user can change, you hit an issue: if FOO_USER changes, the new FOO_USER can't access files created by the old FOO_USER. Unless you know exactly what files he will create, the most obvious way to fix that is to run "chown -R FOO_USER" on all of the directories that the daemon uses at start-up time. But if you do that, then you've accidentally given root to FOO_USER: chown -R will follow sym/hard links, and the FOO_USER can place a symlink in one of those directories pointing e.g. /root/.bashrc. Then the next time you start the daemon, your chown -R will ownership of /root/.bashrc to FOO_USER. You hadn't done any of that, but then again, changing UNREALIRCD_USER didn't work yet =) (I know of no safe way to fix permissions on a bunch of pre-existing files when the runtime user changes, by the way.) You can put it back if you want, but so long as it doesn't work, removing it made the init script a little simpler and future-proof against fixes like "chown -R".
This has been assigned CVE-2017-13649, but upstream isn't going to change the way the PID file is handled, and we've already fixed it in the appropriate way: don't trust the PID file.
Please commit 4.0.13 ebuild. I am assuming we will be making 4.0.13 stable after ebuld is in and dropping 3.2.10.7
@mjo I would like to push your patches to resolve this bug, can you provide certificate of origin per GLEP76?
(In reply to Michael Palimaka (kensington) from comment #13) > @mjo I would like to push your patches to resolve this bug, can you provide > certificate of origin per GLEP76? Sure. You should be able to sign-off on it based on > 4. The contribution was provided directly to me by some other person who certified 1., 2., 3., or 4., and I have not modified it. since I hereby certify that > The contribution was created in whole or in part by me, and I have the right to submit it under the free software license indicated in the file
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=73e4184262bdb92c08fe83d2b9cd06f6a12ccb73 commit 73e4184262bdb92c08fe83d2b9cd06f6a12ccb73 Author: Michael Palimaka <kensington@gentoo.org> AuthorDate: 2018-10-11 12:18:35 +0000 Commit: Michael Palimaka <kensington@gentoo.org> CommitDate: 2018-10-11 12:18:54 +0000 net-irc/unrealircd: revision bump to resolve CVE-2017-13649 Bug: https://bugs.gentoo.org/628434 Signed-off-by: Michael Palimaka <kensington@gentoo.org> Package-Manager: Portage-2.3.49, Repoman-2.3.11 net-irc/unrealircd/files/unrealircd.confd-r3 | 32 +++++ net-irc/unrealircd/files/unrealircd.initd-r2 | 38 +++++ net-irc/unrealircd/unrealircd-4.0.18-r1.ebuild | 184 +++++++++++++++++++++++++ 3 files changed, 254 insertions(+)
This is fixed in unrealircd-4.0.18-r1 which is now stable and all old versions are removed.
Downgraded to C3 as a DoS.
