152 lines
4.7 KiB
Diff
152 lines
4.7 KiB
Diff
From e9fa9637b895293fac8e27deb153d68f1c2b6ab9 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.
|
||
---
|
||
ChangeLog.md | 1 +
|
||
config.c | 7 ++++---
|
||
test/Makefile.am | 4 +++-
|
||
test/test-0102.sh | 16 ++++++++++++++++
|
||
test/test-0103.sh | 16 ++++++++++++++++
|
||
test/test-config.102.in | 10 ++++++++++
|
||
test/test-config.103.in | 12 ++++++++++++
|
||
7 files changed, 62 insertions(+), 4 deletions(-)
|
||
create mode 100755 test/test-0102.sh
|
||
create mode 100755 test/test-0103.sh
|
||
create mode 100644 test/test-config.102.in
|
||
create mode 100644 test/test-config.103.in
|
||
|
||
Index: logrotate-3.18.1/ChangeLog.md
|
||
===================================================================
|
||
--- logrotate-3.18.1.orig/ChangeLog.md
|
||
+++ logrotate-3.18.1/ChangeLog.md
|
||
@@ -5,6 +5,7 @@ All notable changes to this project will
|
||
## [UNRELEASED]
|
||
|
||
[UNRELEASED]: https://github.com/logrotate/logrotate/compare/3.18.1...master
|
||
+ - enforce stricter parsing of configuration files
|
||
|
||
## [3.18.1] - 2021-05-21
|
||
- fix memory leaks on error-handling paths (#383, #387)
|
||
Index: logrotate-3.18.1/config.c
|
||
===================================================================
|
||
--- logrotate-3.18.1.orig/config.c
|
||
+++ logrotate-3.18.1/config.c
|
||
@@ -1095,12 +1095,13 @@ static int readConfigFile(const char *co
|
||
if (key == NULL) {
|
||
message(MESS_ERROR, "%s:%d failed to parse keyword\n",
|
||
configFile, lineNum);
|
||
- continue;
|
||
+ RAISE_ERROR();
|
||
}
|
||
if (!isspace((unsigned char)*start)) {
|
||
- message(MESS_NORMAL, "%s:%d keyword '%s' not properly"
|
||
+ message(MESS_ERROR, "%s:%d keyword '%s' not properly"
|
||
" separated, found %#x\n",
|
||
configFile, lineNum, key, *start);
|
||
+ RAISE_ERROR();
|
||
}
|
||
if (!strcmp(key, "compress")) {
|
||
newlog->flags |= LOG_FLAG_COMPRESS;
|
||
@@ -1978,7 +1979,7 @@ duperror:
|
||
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;
|
||
+ RAISE_ERROR();
|
||
}
|
||
break;
|
||
case STATE_SKIP_LINE:
|
||
Index: logrotate-3.18.1/test/Makefile.am
|
||
===================================================================
|
||
--- logrotate-3.18.1.orig/test/Makefile.am
|
||
+++ logrotate-3.18.1/test/Makefile.am
|
||
@@ -89,7 +89,9 @@ TEST_CASES = \
|
||
test-0088.sh \
|
||
test-0092.sh \
|
||
test-0100.sh \
|
||
- test-0101.sh
|
||
+ test-0101.sh \
|
||
+ test-0102.sh \
|
||
+ test-0103.sh
|
||
|
||
EXTRA_DIST = \
|
||
compress \
|
||
Index: logrotate-3.18.1/test/test-0102.sh
|
||
===================================================================
|
||
--- /dev/null
|
||
+++ logrotate-3.18.1/test/test-0102.sh
|
||
@@ -0,0 +1,16 @@
|
||
+#!/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
|
||
Index: logrotate-3.18.1/test/test-0103.sh
|
||
===================================================================
|
||
--- /dev/null
|
||
+++ logrotate-3.18.1/test/test-0103.sh
|
||
@@ -0,0 +1,16 @@
|
||
+#!/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
|
||
Index: logrotate-3.18.1/test/test-config.102.in
|
||
===================================================================
|
||
--- /dev/null
|
||
+++ logrotate-3.18.1/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.18.1/test/test-config.103.in
|
||
===================================================================
|
||
--- /dev/null
|
||
+++ logrotate-3.18.1/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
|
||
+}
|