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 <

+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