View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0004990 | unreal | ircd | public | 2017-08-21 16:00 | 2022-01-01 01:44 |
| Reporter | mjo | Assigned To | syzop | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | no change required | ||
| Platform | Unix (Linux, BSD) | ||||
| Product Version | 4.0.13 | ||||
| Summary | 0004990: Privilege escalation via PID file manipulation | ||||
| Description | Hi, I've found a minor security vulnerability in the way unrealircd handles its PID file. This is going to get a little bit confusing due to the way the program daemonizes itself, so I apologize in advance. Feel free to tell me to clarify things. In summary, PID files are generally read and acted upon by root, so their contents should only be writable by root. Typically, to stop a daemon, root (by way of the init system) will read the PID file, and call "kill" on it. If a non-root user can write an arbitrary PID into the PID file, then that user can trick root into killing off other processes on the system. The problem as it relates to unrealircd is that, if anyone is actually using the PID file that the daemon creates, it's very difficult to do so correctly. When running in the background, the unrealircd daemon does not drop privileges on its own, and it also refuses to run as root. Thus, the daemon will be started by a non-root user, and the PID file will be created as that non-root user. At that point, if anyone reads the PID file as part of their init script, they're vulnerable to the aforementioned exploit. Now I'll just throw out some background about how programs daemonize. There are two ways in common use. The first way (smart daemon, dumb init system): the program itself is responsible for forking, writing a PID file, calling setsid(), closing stdin/stdout/stderr, dropping privileges, etc. The init system just runs some shell script to coordinate things. This is more portable and easy to understand. The second way (smart init system, dumb program): the program runs in the foreground and doesn't do any of that stuff. Instead, all of the complexity is in the init system, which handles things for you. This is cleaner, but needs a special init system. The first way is used by traditional sysvinit scripts, and the second might as well be called "the systemd way." The issue in unrealircd comes down to the fact that you create a PID file, but you don't do the other part of "the first way" -- namely, you don't drop privileges in the daemon itself. If you did, you could create the PID file as root, and then drop privileges afterwards to avoid the problem. But since you don't, the PID file will be created by the same non-root user who starts unrealircd. Your implementation of "the second way" is fine, because you don't have to do anything =) The maintainer of unrealircd in Gentoo hinted that you might not care too much about the init script use case, so I'll offer two solutions to the problem. 1. If you want to support traditional init scripts: get rid of the "don't run this as root!" warning. Add --user and --group flags to the daemon, and have the daemon drop privileges to that user and group *after* it creates the PID file. 2. If you don't care about traditional init scripts, i.e. if you want to require a more-sophisticated init system like OpenRC or systemd: get rid of the PID file entirely, and eliminate the option for the daemon to go into the background. Those init systems can handle everything on their own. Basically, you just want to eliminate the temptation to use the contents of the PID file. Since it will always be writable by a non-root user, and root is really the only person who would use it, all PID file usage is suspicious. | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | |||||
|
|
Hi, First of all, thank you for creating a bug report. The vulnerability here is that a signal may be sent to the wrong process, correct? As for why we have a pid file in the first place (and it's perfectly fine in the following case): You should know that the majority of UnrealIRCd installations build and install from source. So you have a user 'ircd' which logs in to the shell and they fetch our .tar.gz, ./Config, make, make install into /home/ircd/unrealircd/. Then they will use the script ./unrealircd start and ./unrealircd stop and ./unrealircd rehash and so on. That is the way most (I don't have figures, but I think likely >95%) of our users use UnrealIRCd. Usually crontab is used for starting the daemon on startup, there is a script 'ircdchk' specially for that. It seems that most, if not all, of what you wrote assumes an init-alike system. I can perfectly understand that if you wrote it as a general text. I just wanted to point out in our case it rarely is for that :) Anyway, on to the solutions you proposed. 1. If you want to support traditional init scripts: get rid of the "don't run this as root!" warning. Add --user and --group flags to the daemon, and have the daemon drop privileges to that user and group *after* it creates the PID file. => Allowing to run as root is not going to happen I'm afraid. It would be very easy to add privilege dropping but there would still be (way!) too much code that runs as root. It would be irresponsible. We don't want that and in fact that is precisely the reason we put the "don't run as root" thing in and ripped the --user and --group stuff out. We very much would recommend against anyone changing those restrictions as well (eg: via some distro-specific patch). It would create a security hole that goes far beyond killing wrong processes! 2. If you don't care about traditional init scripts, i.e. if you want to require a more-sophisticated init system like OpenRC or systemd: get rid of the PID file entirely, and eliminate the option for the daemon to go into the background. Those init systems can handle everything on their own. => So those systems can deal with it, no need to do anything from our side, from what I understand? Great, definitely recommended then. As for removing PID file support: see my first section as to why pid is useful for us -- and safe, all the starting, stopping etc happens under the same user. No root involved. I think there's also an option for it still to work with traditional init that you didn't bring up: => The "stop" script could check if the PID belong to the user running the daemon. Say, you run UnrealIRCd as user 'irc', then the PID should also belong to 'irc' (and not 'root' or 'avahi' or whatever). Of course, this assumes you run the ircd from a dedicated account like 'irc' (and not 'daemon' or something that also runs other programs). That is, in fact, something we always very much recommend. Most *NIX systems assume this too I think, as I see 'irc' and 'ircd' user accounts created on those systems. Anyway, your initial assumption is correct that we at UnrealIRCd don't deal with init scripts (we don't ship any either). We leave that kind of stuff up to the distro. I'm happy to send out an emailabout a potential security problem. Telling users or maintainers to take a look at one thing or another. We always do our best to get the best security for our users. Even if better security bites us (such as in this "don't run as root" case). Out of curiosity, was this email sent as an independent researcher or are you the gentoo maintainer of UnrealIRCd? Regards, Bram |
|
|
Just a small note. What I meant with: "Allowing to run as root is not going to happen I'm afraid. It would be very easy to add but there would be (way!) too much code that runs as root. " Is: "Allowing to run as root is not going to happen I'm afraid. It would be very easy to add privilege dropping but there would still be (way!) too much code that runs as root. " The rest still stands :) |
|
|
> The vulnerability here is that a signal may be sent to the wrong process, correct? Correct. > Allowing to run as root is not going to happen I'm afraid. It would be very easy to add but there would be (way!) too much code that runs as root. It would be irresponsible. Don't worry, I wasn't suggesting that you actually let the daemon continue to run as root =) However, in order to drop privileges (i.e. to change your uid), you need to be root, because he's the only one that can do that. Instead of a "don't run this as root!" warning, you might instead have a "the --username flag is required!" warning, so that the daemon is guaranteed to drop privileges immediately after starting. > The "stop" script could check if the PID belong to the user running the daemon. Ah, a great idea -- but one that I've discovered is next to impossible to do in a portable manner. Your ./unrealircd script actually has the same problem that I found in our init script. It trusts the PID file's contents, but it's not guaranteed to be run by the same user that owns the PID file. If I run "./unrealircd start" as root, then of course I will eventually get the warning; but "./unrealircd stop" runs just fine as root, and kills whatever the PID file contains. Thus if I run "./unrealircd start" as one user and "./unrealircd stop" as root, the wrong process can get killed. You're probably thinking: why the hell would you run "stop" as root? Easy: people are dumb. Adding "sudo" to the beginning of a command is how you make it work =) The right thing to do there is to check that the owner of the unrealircd daemon process is the same as the user sending the signal is the same as the owner of the PID file. But I've tried to implement that in shell script and failed. I've also never seen it done successfully in another package. There are a select few that manage to do it, like PostgreSQL's pg_ctl tool, but they've implemented it in C code. > Out of curiosity, was this email sent as an independent researcher or are you the gentoo maintainer of UnrealIRCd? A little of both. I'm a Gentoo developer, but not the maintainer of unrealircd. I've found similar vulnerabilities in other packages, and I rewrote our vulnerable OpenRC init script to avoid the problem by running the daemon in the foreground. > I'm happy to send out an email about a potential security problem. I wouldn't worry about it yet, the risk is minor because a previous exploit is needed to do anything bad. If you decide not to address this in the daemon (or in the ./unrealircd script), then I'll eventually file a CVE for it and try to track down all of the init scripts I can find. The Gentoo one I've already fixed, but a quick google search for "unrealircd init script" turns up a lot more, and it looks like they all do pretty much the same thing. |
|
|
> However, in order to drop privileges (i.e. to change your uid), you need to be root, because he's the only one that can do that. Instead of a "don't run this as root!" warning, you might instead have a "the --username flag is required!" warning, so that the daemon is guaranteed to drop privileges immediately after starting. Yes, I understand (sorry you didn't see the correction in time :D). What I mean is that way too much code runs as root then, such as configuration code. This is not what we want. > Ah, a great idea -- but one that I've discovered is next to impossible to do in a portable manner. Ah okay. Not sure about portable, but I would say on Linux this should be fairly easy, no? KILLPIDS="`cat ......pid` su -s /bin/sh -c "kill -15 -- $KILLPIDS" irc Well, you get the idea :) Anyway, we aren't providing init scripts, so I probably shouldn't provide suggestions for it either :) > If you decide not to address this in the daemon (or in the ./unrealircd script), then I'll eventually file a CVE for it and try to track down all of the init scripts I can find. The Gentoo one I've already fixed, but a quick google search for "unrealircd init script" turns up a lot more, and it looks like they all do pretty much the same thing. This sounds fine. I'm only aware of gentoo and archlinux that package UnrealIRCd. I'm not aware of any others. Will you contact arch linux? I'm asking explicitly so there is no miscommunication in this :D. Like I said, our users generally use cron to start the daemon in the self-install scenario, rather than using init. We refer to this from https://www.unrealircd.org/docs/Installing_from_source to https://www.unrealircd.org/docs/Cron_job As for users running "./unrealircd stop" as root.. if users do this in the self-install scenario (=the only ones we support ourselves) then they are already running the "unrealircd" script that is owned by <non-root> so they are screwed up anyway since it may contain any code.. nothing we can do. |
|
|
> Will you contact arch linux? I'm asking explicitly so there is no miscommunication in this Yes, I'll contact everyone that I'm able to identify. I'll also post the details and the suggested workaround to the oss-security list. All major distributions have someone watching that list for announcements that affect them. Arch is now using systemd so the fix is easy for them: * add the "-F" flag to run the daemon in the foreground * drop the Type=forking line from the service file, because now it won't fork * drop the PIDFile=... line from the service file * drop the unrealircd.tmpfiles.d file The solution for OpenRC is similarly pleasing. For the recommended workaround, I'll suggest something like the above for init systems that can background and manage the daemon on their own. For SysV init (and other systems that can't), I will probably just suggest that they not run unrealircd via an init script. |
|
|
Great. Thanks! Then this report can be closed. No change required on our end. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2017-08-21 16:00 | mjo | New Issue | |
| 2017-08-21 17:35 | syzop | Note Added: 0019826 | |
| 2017-08-21 17:36 | syzop | Note Added: 0019827 | |
| 2017-08-21 17:57 | syzop | Note Edited: 0019826 | |
| 2017-08-21 18:32 | mjo | Note Added: 0019828 | |
| 2017-08-21 18:55 | syzop | Note Added: 0019829 | |
| 2017-08-21 19:35 | mjo | Note Added: 0019830 | |
| 2017-08-21 20:10 | syzop | Assigned To | => syzop |
| 2017-08-21 20:10 | syzop | Status | new => closed |
| 2017-08-21 20:10 | syzop | Resolution | open => no change required |
| 2017-08-21 20:10 | syzop | Note Added: 0019831 |
