Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 628546 - <app-admin/filebeat-5.5.2: DoS via PID file manipulation
Summary: <app-admin/filebeat-5.5.2: DoS via PID file manipulation
Alias: None
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo Security
Whiteboard: B3 [noglsa]
Depends on:
Reported: 2017-08-21 19:05 UTC by Michael Orlitzky
Modified: 2017-09-04 18:53 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---
stable-bot: sanity-check+


Note You need to log in before you can comment on or make changes to this bug.
Description Michael Orlitzky gentoo-dev 2017-08-21 19:05:29 UTC
The init script for filebeat gives ownership of the PID file directory to the same user that the daemon runs as:

  checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}"
            -m750 "$(dirname "${pidfile}")"

As a result, the ${FILEBEAT_USER} user can write whatever he wants into the PID file. Later, that may be exploitable: when the service is stopped, root will call "kill" on the contents of that file.

But there's good news: there's absolutely no reason for the call to "checkpath" above. With command_background="true", OpenRC creates the PID file as root:root, and the problematic "checkpath" can be deleted if you store the PID file directly in /run.

Some other minor improvements to consider while you're touching the init script:

  1. Utilize command_user for the user/group instead of start_stop_daemon_args
  2. Can the OpenRC "retry" variable be used to eliminate stop() ?

This isn't a very big deal since ${FILEBEAT_USER} and ${FILEBEAT_GROUP} default to root. However, if there's a benefit to running as a restricted user, maybe now's a good time to rethink the way it's done?

Rather than let the user/group be variable, I would suggest creating a dedicated user/group in the ebuild, and then hard-coding those in the init/conf scripts. The reason is, if you let the end-user change FILEBEAT_USER, you will quickly hit an issue: your files might be owned by the old user, and inaccessible to the new one. To work around that, you'll come up with some recursive chown thing, and introduce a new problem. (This is all tangential; just something to keep in mind.) If the user/group were hard-coded, the other two checkpath calls would also be redundant.
Comment 1 Tomáš Mózes 2017-08-22 12:51:57 UTC
Thanks for sharing your finding Michael. The user/group is configurable because filebeat is made mostly for reading logs, some are owner by root, some may be owner by unprivileged users. Therefore I think it's nice to be able to set the user. The problem is that we cannot play nice when changing users without posing a security risk. To be on the safe side, the ebuild should note, that changing users is to be processed by the user itself, however they may be times we switch the default user and it may break user installations. I'm working on an improved version of the init script, will link a PR here (hopefully soon). So far it looks like this:

# diff -u filebeat.initd /etc/init.d/filebeat
--- filebeat.initd      2017-02-12 17:03:04.000000000 +0100
+++ /etc/init.d/filebeat        2017-08-22 14:46:05.205668639 +0200
@@ -12,11 +12,12 @@
 command_args="-c ${FILEBEAT_CONFIG} ${FILEBEAT_OPTS} -path.config $(dirname $FILEBEAT_CONFIG) \ ${FILEBEAT_DATADIR} -path.home ${FILEBEAT_DATADIR} -path.logs ${FILEBEAT_LOGDIR}"
-start_stop_daemon_args="--user ${FILEBEAT_USER}:${FILEBEAT_GROUP} \
-       --chdir ${FILEBEAT_DATADIR}"
+start_stop_daemon_args="--chdir ${FILEBEAT_DATADIR}"
 depend() {
        use net
@@ -37,14 +38,6 @@
 start_pre() {
        checkconfig || return 1
-       checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "$(dirname "${pidfile}")"
        checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "${FILEBEAT_DATADIR}"
        checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "${FILEBEAT_LOGDIR}"
-stop() {
-       ebegin "Stopping filebeat"
-       start-stop-daemon --stop \
-               --pidfile=${pidfile} \
-               --retry=TERM/5/KILL/5
Comment 2 Tomáš Mózes 2017-08-23 07:17:11 UTC

We can then stabilize version 5.5.2.
Comment 3 Tomáš Mózes 2017-09-04 06:49:54 UTC
Please stabilize app-admin/filebeat-5.5.2 on amd64 and x86. Thanks.
Comment 4 Christopher Díaz Riveros (RETIRED) gentoo-dev Security 2017-09-04 14:40:38 UTC
(In reply to Tomáš Mózes from comment #3)
> Please stabilize app-admin/filebeat-5.5.2 on amd64 and x86. Thanks.

Thank you, since there was no previous stable version on x86, that should go on a new report, we can stabilize arches that already were stable before the report. I'll call amd64 for stabilization.

@Arch, please test and mark stable.

Gentoo Security Padawan
Comment 5 Aaron Bauman (RETIRED) gentoo-dev 2017-09-04 18:53:15 UTC
amd64 stable

GLSA Vote: No