From 53e0dc4a8ddcb169b0ba36472de03f4366f45159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 29 Mar 2022 21:06:54 +0200 Subject: [PATCH 1/3] skip locking if state file is world-readable Fixes: CVE-2022-1348 - potential DoS from unprivileged users via the state file Bug: https://bugzilla.redhat.com/CVE-2022-1348 Upstream-commit: 1f76a381e2caa0603ae3dbc51ed0f1aa0d6658b9 Signed-off-by: Kamil Dudka --- logrotate.c | 24 ++++++++++++++++++++++-- logrotate.spec.in | 3 +-- test/Makefile.am | 1 + test/test-0087.sh | 1 + test/test-0092.sh | 19 +++++++++++++++++++ test/test-config.92.in | 4 ++++ 6 files changed, 48 insertions(+), 4 deletions(-) create mode 100755 test/test-0092.sh create mode 100644 test/test-config.92.in diff --git a/logrotate.c b/logrotate.c index d3f2825..78153b3 100644 --- a/logrotate.c +++ b/logrotate.c @@ -2565,6 +2565,9 @@ static int writeState(const char *stateFilename) close(fdcurr); + /* drop world-readable flag to prevent others from locking */ + sb.st_mode &= ~(mode_t)S_IROTH; + fdsave = createOutputFile(tmpFilename, O_RDWR | O_CREAT | O_TRUNC, &sb, prev_acl, 0); #ifdef WITH_ACL if (prev_acl) { @@ -2898,15 +2901,16 @@ static int readState(const char *stateFilename) static int lockState(const char *stateFilename, int skip_state_lock) { + struct stat sb; int lockFd = open(stateFilename, O_RDWR | O_CLOEXEC); if (lockFd == -1) { if (errno == ENOENT) { message(MESS_DEBUG, "Creating stub state file: %s\n", stateFilename); - /* create a stub state file with mode 0644 */ + /* create a stub state file with mode 0640 */ lockFd = open(stateFilename, O_CREAT | O_EXCL | O_WRONLY, - S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH); + S_IWUSR | S_IRUSR | S_IRGRP); if (lockFd == -1) { message(MESS_ERROR, "error creating stub state file %s: %s\n", stateFilename, strerror(errno)); @@ -2926,6 +2930,22 @@ static int lockState(const char *stateFilename, int skip_state_lock) return 0; } + if (fstat(lockFd, &sb) == -1) { + message(MESS_ERROR, "error stat()ing state file %s: %s\n", + stateFilename, strerror(errno)); + close(lockFd); + return 1; + } + + if (sb.st_mode & S_IROTH) { + message(MESS_ERROR, "state file %s is world-readable and thus can" + " be locked from other unprivileged users." + " Skipping lock acquisition...\n", + stateFilename); + close(lockFd); + return 0; + } + if (flock(lockFd, LOCK_EX | LOCK_NB) == -1) { if (errno == EWOULDBLOCK) { message(MESS_ERROR, "state file %s is already locked\n" diff --git a/logrotate.spec.in b/logrotate.spec.in index 92e1d97..3caabf2 100644 --- a/logrotate.spec.in +++ b/logrotate.spec.in @@ -41,7 +41,6 @@ install -p -m 644 examples/logrotate.conf $RPM_BUILD_ROOT%{_sysconfdir}/logrotat install -p -m 644 examples/btmp $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/btmp install -p -m 644 examples/wtmp $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/wtmp install -p -m 755 examples/logrotate.cron $RPM_BUILD_ROOT%{_sysconfdir}/cron.daily/logrotate -touch $RPM_BUILD_ROOT%{_localstatedir}/lib/logrotate.status %clean rm -rf $RPM_BUILD_ROOT @@ -55,4 +54,4 @@ rm -rf $RPM_BUILD_ROOT %attr(0755, root, root) %{_sysconfdir}/cron.daily/logrotate %attr(0644, root, root) %config(noreplace) %{_sysconfdir}/logrotate.conf %attr(0755, root, root) %{_sysconfdir}/logrotate.d -%attr(0644, root, root) %verify(not size md5 mtime) %config(noreplace) %{_localstatedir}/lib/logrotate.status +%ghost %attr(0640, root, root) %verify(not size md5 mtime) %{_localstatedir}/lib/logrotate.status diff --git a/test/Makefile.am b/test/Makefile.am index 914fe65..d6fb7c8 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -87,6 +87,7 @@ TEST_CASES = \ test-0086.sh \ test-0087.sh \ test-0088.sh \ + test-0092.sh \ test-0100.sh \ test-0101.sh diff --git a/test/test-0087.sh b/test/test-0087.sh index 91e5266..aeff2c6 100755 --- a/test/test-0087.sh +++ b/test/test-0087.sh @@ -8,6 +8,7 @@ cleanup 87 preptest test.log 87 1 touch state +chmod 0640 state $RLR test-config.87 -f & diff --git a/test/test-0092.sh b/test/test-0092.sh new file mode 100755 index 0000000..be52e14 --- /dev/null +++ b/test/test-0092.sh @@ -0,0 +1,19 @@ +#!/bin/sh + +. ./test-common.sh + +# check state file locking +cleanup 92 + +preptest test.log 92 1 + +touch state +chmod 0644 state +flock state -c "sleep 10" & + +$RLR -f test-config.92 || exit 23 + +checkoutput < Date: Wed, 25 May 2022 09:55:02 +0200 Subject: [PATCH 2/3] drop world-readable permission on state file ... even when ACLs are enabled. This is a follow-up to the fix of CVE-2022-1348. It has no impact on security but makes the state file locking work again in more cases. Closes: https://github.com/logrotate/logrotate/pull/446 Upstream-commit: addbd293242b0b78aa54f054e6c1d249451f137d Signed-off-by: Kamil Dudka --- logrotate.c | 10 +++++++--- test/test-0048.sh | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/logrotate.c b/logrotate.c index 78153b3..8d49f26 100644 --- a/logrotate.c +++ b/logrotate.c @@ -2498,6 +2498,7 @@ static int writeState(const char *stateFilename) struct tm now; time_t now_time, last_time; char *prevCtx; + int force_mode = 0; localtime_r(&nowSecs, &now); @@ -2565,10 +2566,13 @@ static int writeState(const char *stateFilename) close(fdcurr); - /* drop world-readable flag to prevent others from locking */ - sb.st_mode &= ~(mode_t)S_IROTH; + if (sb.st_mode & (mode_t)S_IROTH) { + /* drop world-readable flag to prevent others from locking */ + sb.st_mode &= ~(mode_t)S_IROTH; + force_mode = 1; + } - fdsave = createOutputFile(tmpFilename, O_RDWR | O_CREAT | O_TRUNC, &sb, prev_acl, 0); + fdsave = createOutputFile(tmpFilename, O_RDWR | O_CREAT | O_TRUNC, &sb, prev_acl, force_mode); #ifdef WITH_ACL if (prev_acl) { acl_free(prev_acl); diff --git a/test/test-0048.sh b/test/test-0048.sh index 62d606b..06b255a 100755 --- a/test/test-0048.sh +++ b/test/test-0048.sh @@ -18,6 +18,7 @@ cat > state << EOF logrotate state -- version 2 EOF +chmod 0640 state setfacl -m u:nobody:rwx state $RLR test-config.48 -- 2.35.3 From 105ed9f433a3aaf1aec93318aa9c8811b59d7b23 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Fri, 27 May 2022 09:56:07 +0200 Subject: [PATCH 3/3] lockState: do not print `error:` when exit code is unaffected Closes: https://github.com/logrotate/logrotate/pull/448 Upstream-commit: 31cf1099ab8514dfcae5a980bc77352edd5292f8 Signed-off-by: Kamil Dudka --- logrotate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/logrotate.c b/logrotate.c index 27deaf3..77db8c2 100644 --- a/logrotate.c +++ b/logrotate.c @@ -2942,8 +2942,8 @@ static int lockState(const char *stateFilename, int skip_state_lock) } if (sb.st_mode & S_IROTH) { - message(MESS_ERROR, "state file %s is world-readable and thus can" - " be locked from other unprivileged users." + message(MESS_NORMAL, "warning: state file %s is world-readable" + " and thus can be locked from other unprivileged users." " Skipping lock acquisition...\n", stateFilename); close(lockFd); -- 2.35.3