File logrotate-enforce-stricter-parsing-and-extra-tests.patch of Package logrotate.24944
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.13.0/config.c
===================================================================
--- logrotate-3.13.0.orig/config.c
+++ logrotate-3.13.0/config.c
@@ -1023,8 +1023,17 @@ static int readConfigFile(const char *co
}
if (isalpha((unsigned char)*start)) {
- if ((key = isolateWord(&start, &buf, length)) == NULL)
- continue;
+ if ((key = isolateWord(&start, &buf, length)) == NULL) {
+ message(MESS_ERROR, "%s:%d failed to parse keyword\n",
+ configFile, lineNum);
+ goto error;
+ }
+ if (!isspace((unsigned char)*start)) {
+ message(MESS_ERROR, "%s:%d keyword '%s' not properly"
+ " separated, found %#x\n",
+ configFile, lineNum, key, *start);
+ goto error;
+ }
if (!strcmp(key, "compress")) {
newlog->flags |= LOG_FLAG_COMPRESS;
} else if (!strcmp(key, "nocompress")) {
@@ -1770,7 +1779,7 @@ static int readConfigFile(const char *co
message(MESS_ERROR, "%s:%d lines must begin with a keyword "
"or a filename (possibly in double quotes)\n",
configFile, lineNum);
- state = STATE_SKIP_LINE;
+ goto error;
}
break;
case STATE_SKIP_LINE:
Index: logrotate-3.13.0/test/test-config.102.in
===================================================================
--- /dev/null
+++ logrotate-3.13.0/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.13.0/test/test-config.103.in
===================================================================
--- /dev/null
+++ logrotate-3.13.0/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.13.0/test/test-config.104.in
===================================================================
--- /dev/null
+++ logrotate-3.13.0/test/test-config.104.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+ newkeyword
+ rotate 1
+}
+
+&DIR&/test2.log {
+ rotate 1
+}
Index: logrotate-3.13.0/test/test-config.105.in
===================================================================
--- /dev/null
+++ logrotate-3.13.0/test/test-config.105.in
@@ -0,0 +1,8 @@
+&DIR&/test1.log {
+ g@rbag€[]+#*
+ rotate 1
+}
+
+&DIR&/test2.log {
+ rotate 1
+}
\ No newline at end of file
Index: logrotate-3.13.0/test/test-0102.sh
===================================================================
--- /dev/null
+++ logrotate-3.13.0/test/test-0102.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+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
+test.log.1 0 first
+EOF
Index: logrotate-3.13.0/test/test-0103.sh
===================================================================
--- /dev/null
+++ logrotate-3.13.0/test/test-0103.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+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
+test.log.1 0 first
+EOF
Index: logrotate-3.13.0/test/test-0104.sh
===================================================================
--- /dev/null
+++ logrotate-3.13.0/test/test-0104.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+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
Index: logrotate-3.13.0/test/test-0105.sh
===================================================================
--- /dev/null
+++ logrotate-3.13.0/test/test-0105.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+. ./test-common.sh
+
+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
+test2.log 0 zero
+EOF
Index: logrotate-3.13.0/test/Makefile.am
===================================================================
--- logrotate-3.13.0.orig/test/Makefile.am
+++ logrotate-3.13.0/test/Makefile.am
@@ -76,7 +76,11 @@ TEST_CASES = \
test-0075.sh \
test-0076.sh \
test-0100.sh \
- test-0101.sh
+ test-0101.sh \
+ test-0102.sh \
+ test-0103.sh \
+ test-0104.sh \
+ test-0105.sh
EXTRA_DIST = \
compress \
