From 53e0dc4a8ddcb169b0ba36472de03f4366f45159 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
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 <kdudka@redhat.com>
---
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 <<EOF
+test.log 0
+test.log.1 0 zero
+EOF
diff --git a/test/test-config.92.in b/test/test-config.92.in
new file mode 100644
index 0000000..ac93900
--- /dev/null
+++ b/test/test-config.92.in
@@ -0,0 +1,4 @@
+&DIR&/test.log {
+ rotate 1
+ create
+}
--
2.35.3
From 0d2d770cc5aa7bf14e84a2832249eeeb391b0b8a Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
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 <kdudka@redhat.com>
---
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 <kdudka@redhat.com>
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 <kdudka@redhat.com>
---
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