From c7c4ad3880a966cae2e3bb021b861e4c3dfa90f9 Mon Sep 17 00:00:00 2001
From: Felix Wilhelm <fwilhelm@google.com>
Date: Thu, 21 Oct 2021 09:47:57 +0000
Subject: [PATCH] config.c: enforce stricter parsing of config files

Abort parsing of config files that contain invalid lines.
This makes it harder to abuse logrotate for privilege escalation
attacks where an attacker can partially control a privileged file write.

[iluceno@suse.de: Backported from 3.19.0]
Ref: 124e4ca6532b ("config.c: enforce stricter parsing of config files")
Ref: 9cbc22b91caf ("Add more testcases for stricter configuration parsing")
Signed-off-by: Ismael Luceno <iluceno@suse.de>
---
 config.c                | 15 +++++++--
 test/test               | 69 +++++++++++++++++++++++++++++++++++++++++
 test/test-config.102.in | 10 ++++++
 test/test-config.103.in | 12 +++++++
 test/test-config.104.in |  8 +++++
 test/test-config.105.in |  8 +++++
 6 files changed, 119 insertions(+), 3 deletions(-)
 create mode 100644 test/test-config.102.in
 create mode 100644 test/test-config.103.in
 create mode 100644 test/test-config.104.in
 create mode 100644 test/test-config.105.in

Index: logrotate-3.7.7/config.c
===================================================================
--- logrotate-3.7.7.orig/config.c
+++ logrotate-3.7.7/config.c
@@ -632,6 +632,17 @@ static int readConfigFile(const char *co
 	    oldchar = *endtag;
 	    *endtag = '\0';
 
+	    if (oldchar == '\0') {
+		message(MESS_ERROR, "%s:%d failed to parse keyword\n",
+			configFile, lineNum);
+		return 1;
+	    }
+	    if (!isspace((unsigned char)oldchar) && oldchar != '=') {
+		message(MESS_ERROR, "%s:%d keyword '%s' not properly"
+			" separated, found %#x\n",
+			configFile, lineNum, start, oldchar);
+		return 1;
+	    }
 	    if (!strcmp(start, "compress")) {
 		newlog->flags |= LOG_FLAG_COMPRESS;
 
@@ -1544,11 +1555,7 @@ duperror:
 	    message(MESS_ERROR, "%s:%d lines must begin with a keyword "
 		    "or a filename (possibly in double quotes)\n",
 		    configFile, lineNum);
-
-	    while (*start != '\n')
-		start++;
-	    lineNum++;
-	    start++;
+	    return 1;
 	}
     }
 
Index: logrotate-3.7.7/test/test
===================================================================
--- logrotate-3.7.7.orig/test/test
+++ logrotate-3.7.7/test/test
@@ -594,4 +594,75 @@ fi
 
 rm -f *test.log*
 
+cleanup 102
+
+# ------------------------------- Test 102 ------------------------------------
+# test invalid config file with binary content
+preptest test.log 102 1
+
+$RLR test-config.102 --force
+
+if [ $? -eq 0 ]; then
+   echo "No error, but there should be one."
+  exit 3
+fi
+
+checkoutput <<EOF
+test.log 0 zero
+EOF
+
+cleanup 103
+
+# ------------------------------- Test 103 ------------------------------------
+# test invalid config file with unknown keywords
+preptest test.log 103 1
+
+$RLR test-config.103 --force
+
+if [ $? -eq 0 ]; then
+   echo "No error, but there should be one."
+   exit 3
+fi
+
+checkoutput <<EOF
+test.log 0 zero
+EOF
+
+cleanup 104
+
+# ------------------------------- Test 104 ------------------------------------
+# test config with unknown (new?) keyword
+preptest test1.log 104 1
+preptest test2.log 104 1
+
+$RLR test-config.104 --force || exit 23
+
+checkoutput <<EOF
+test1.log 0
+test1.log.1 0 zero
+test2.log 0
+test2.log.1 0 zero
+EOF
+
+cleanup 105
+
+# ------------------------------- Test 105 ------------------------------------
+# test config with garbage keyword bails out
+preptest test1.log 105 1
+preptest test2.log 105 1
+
+$RLR test-config.105 --force
+
+if [ $? -eq 0 ]; then
+   echo "No error, but there should be one."
+   exit 3
+fi
+
+
+checkoutput <<EOF
+test1.log 0 zero
+test2.log 0 zero
+EOF
+
 cleanup
+
Index: logrotate-3.7.7/test/test-config.102.in
===================================================================
--- /dev/null
+++ logrotate-3.7.7/test/test-config.102.in
@@ -0,0 +1,10 @@
+ELF
+
+&DIR&/test.log {
+    daily
+    size=0
+
+    firstaction
+    /bin/sh -c "echo test123"
+    endscript
+}
Index: logrotate-3.7.7/test/test-config.103.in
===================================================================
--- /dev/null
+++ logrotate-3.7.7/test/test-config.103.in
@@ -0,0 +1,12 @@
+random noise
+a b c d
+a::x
+
+&DIR&/test.log {
+    daily
+    size=0
+
+    firstaction
+    /bin/sh -c "echo test123"
+    endscript
+}
Index: logrotate-3.7.7/test/test-config.104.in
===================================================================
--- /dev/null
+++ logrotate-3.7.7/test/test-config.104.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+    newkeyword
+    rotate 1
+}
+
+&DIR&/test2.log {
+    rotate 1
+}
Index: logrotate-3.7.7/test/test-config.105.in
===================================================================
--- /dev/null
+++ logrotate-3.7.7/test/test-config.105.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+    g@rbag€[]+#*
+    rotate 1
+}
+
+&DIR&/test2.log {
+    rotate 1
+}
