Attachment 860733 Details for Bug 1192449

[patch] PR#427 backport to 3.7.7

config.c-enforce-stricter-parsing-of-config-files.patch (text/plain), 4.82 KB, created by João Silva on 2022-08-10 15:11:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: João Silva

Created: 2022-08-10 15:11:39 UTC

Size: 4.82 KB

patch

obsolete

>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
>+}

Actions: View | Diff

Attachments on bug 1192449: 854745 | 854746 | 860733