From 1246459dd406fd4e96beb48bcd6ce116d2f5ff34 Mon Sep 17 00:00:00 2001 From: "Scott R. Shinn" Date: Thu, 15 Oct 2026 13:10:37 -0400 Subject: [PATCH 1/2] Preserve syscheck report_changes newlines under control-char strip. CVE-2020-8445 stripping removed the '\n' that frames report_changes diffs, so DecodeSyscheck glued path and diff and raised rule 554. Keep newlines (and tabs) for syscheck locations only. Closes issue #2360 Signed-off-by: Scott R. Shinn --- src/analysisd/cleanevent.c | 10 ++++++++-- src/headers/file_op.h | 4 ++++ src/shared/file_op.c | 29 ++++++++++++++++++++++------- src/shared/tests/Makefile | 9 ++++++++- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/analysisd/cleanevent.c b/src/analysisd/cleanevent.c index 72c553d46..dbad9f269 100644 --- a/src/analysisd/cleanevent.c +++ b/src/analysisd/cleanevent.c @@ -76,8 +76,14 @@ int OS_CleanMSG_ex(char *msg, Eventinfo *lf, time_t recv_time, int update_time_g os_strdup(msg, lf->location); - /*CVE-2020-8445 fix*/ - remove_control_characters(pieces); + /* CVE-2020-8445: strip C0/DEL from the log body. Syscheck locations keep + * '\n'/'\\t' so report_changes (and ACL) framing survives DecodeSyscheck + * (#2359). */ + { + int preserve_nl = (lf->location && strstr(lf->location, "syscheck") != NULL); + + remove_control_characters_ex(pieces, preserve_nl); + } /* Get the log length */ loglen = strlen(pieces) + 1; diff --git a/src/headers/file_op.h b/src/headers/file_op.h index 1055787b1..4e9c118d8 100644 --- a/src/headers/file_op.h +++ b/src/headers/file_op.h @@ -79,5 +79,9 @@ int os_win_network_path(const char *path); void remove_control_characters(char *str); +/* When preserve_nl is non-zero, '\\n' and '\\t' are kept (syscheck + * report_changes / ACL framing). Other C0 and DEL are still removed. */ +void remove_control_characters_ex(char *str, int preserve_nl); + bool is_control_character(char c); #endif /* __FILE_H */ diff --git a/src/shared/file_op.c b/src/shared/file_op.c index 472dc30a9..f761e19b3 100644 --- a/src/shared/file_op.c +++ b/src/shared/file_op.c @@ -1807,23 +1807,38 @@ int w_ref_parent_folder(const char * path) { } bool is_control_character(char c) { - return (c >= 0 && c <= 31) || c == 127 || c == '\n'; + unsigned char uc = (unsigned char)c; + + /* C0 controls and DEL. '\n' is included (uc == 10). */ + return (uc <= 31) || (uc == 127); } -void remove_control_characters(char *str) { +void remove_control_characters_ex(char *str, int preserve_nl) { int len = strlen(str); int i, j; for (i = 0, j = 0; i < len; i++) { - if (!is_control_character(str[i])) { - str[j] = str[i]; - j++; + char c = str[i]; + + /* Syscheck report_changes / Windows ACL text use '\n' as the + * path/diff separator (#2359). Keep tab too for diff bodies. */ + if (preserve_nl && (c == '\n' || c == '\t')) { + str[j++] = c; + continue; + } + + if (!is_control_character(c)) { + str[j++] = c; } else { - DEBUG_MSG("%s:DEBUG: control character removed: %d",ARGV0, str[i]); + DEBUG_MSG("%s:DEBUG: control character removed: %d", ARGV0, (unsigned char)c); } } - str[j] = '\0'; // Null-terminate the string after removing control characters + str[j] = '\0'; +} + +void remove_control_characters(char *str) { + remove_control_characters_ex(str, 0); } #ifdef WIN32 diff --git a/src/shared/tests/Makefile b/src/shared/tests/Makefile index 4a08ade8e..8b48c14ef 100644 --- a/src/shared/tests/Makefile +++ b/src/shared/tests/Makefile @@ -8,7 +8,7 @@ TOP = ../.. CFLAGS_COMMON = -g -Wall -I$(TOP)/headers -I$(TOP) -I$(TOP)/analysisd # Headless tests run by `check`. Interactive helpers stay under `legacy_helpers`. -AUTO_TEST_BINS = thread_pool_test queue_op_test string_test fim_sum_test ar_expect_test fim_maint_test win_acl_test +AUTO_TEST_BINS = thread_pool_test queue_op_test string_test fim_sum_test ar_expect_test fim_maint_test win_acl_test report_changes_ctrl_test LEGACY_HELPER_BINS = prime_test hash_test merge_test ip_test .PHONY: maketest check clean legacy_helpers @@ -45,6 +45,13 @@ fim_maint_test: fim_maint_test.c $(TOP)/shared.a $(TOP)/os_xml.a $(TOP)/os_zlib.a $(TOP)/libcJSON.a -Wl,--end-group \ -lpcre2-8 -lm -lpthread -lz +# #2359: report_changes '\\n' must survive remove_control_characters(). +report_changes_ctrl_test: report_changes_ctrl_test.c $(TOP)/shared.a + $(CC) $(CFLAGS_COMMON) -DARGV0=\"report_changes_ctrl_test\" -o $@ \ + report_changes_ctrl_test.c \ + -Wl,--start-group $(TOP)/shared.a $(TOP)/os_regex.a -Wl,--end-group \ + -lpcre2-8 -lm -lpthread + # Interactive / argv-driven helpers (not part of automated check). legacy_helpers: $(CC) -g -I$(TOP)/headers -I$(TOP) -DARGV0=\"prime_test\" -o prime_test \ From be7e1968018aad0f5e21f49085cc2b8cd1c973ec Mon Sep 17 00:00:00 2001 From: "Scott R. Shinn" Date: Thu, 15 Oct 2026 14:12:20 -0400 Subject: [PATCH 2/2] Code review updates Signed-off-by: Scott R. Shinn --- src/analysisd/cleanevent.c | 6 +----- src/headers/file_op.h | 4 ++++ src/shared/file_op.c | 32 +++++++++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/analysisd/cleanevent.c b/src/analysisd/cleanevent.c index dbad9f269..acbc53858 100644 --- a/src/analysisd/cleanevent.c +++ b/src/analysisd/cleanevent.c @@ -79,11 +79,7 @@ int OS_CleanMSG_ex(char *msg, Eventinfo *lf, time_t recv_time, int update_time_g /* CVE-2020-8445: strip C0/DEL from the log body. Syscheck locations keep * '\n'/'\\t' so report_changes (and ACL) framing survives DecodeSyscheck * (#2359). */ - { - int preserve_nl = (lf->location && strstr(lf->location, "syscheck") != NULL); - - remove_control_characters_ex(pieces, preserve_nl); - } + remove_control_characters_ex(pieces, OS_IsSyscheckLocation(lf->location)); /* Get the log length */ loglen = strlen(pieces) + 1; diff --git a/src/headers/file_op.h b/src/headers/file_op.h index 4e9c118d8..910047e12 100644 --- a/src/headers/file_op.h +++ b/src/headers/file_op.h @@ -83,5 +83,9 @@ void remove_control_characters(char *str); * report_changes / ACL framing). Other C0 and DEL are still removed. */ void remove_control_characters_ex(char *str, int preserve_nl); +/* True for local "syscheck" / "syscheck-registry", or the module after the + * last "->" in an agent location ("(name) ip->syscheck"). */ +int OS_IsSyscheckLocation(const char *location); + bool is_control_character(char c); #endif /* __FILE_H */ diff --git a/src/shared/file_op.c b/src/shared/file_op.c index f761e19b3..96675d4a2 100644 --- a/src/shared/file_op.c +++ b/src/shared/file_op.c @@ -1813,10 +1813,40 @@ bool is_control_character(char c) { return (uc <= 31) || (uc == 127); } +int OS_IsSyscheckLocation(const char *location) +{ + const char *mod; + const char *p; + const char *arrow = NULL; + + if (!location || *location == '\0') { + return (0); + } + + /* Agent locations are "(name) ip->module". Use the last "->" so a + * crafted name containing "->" cannot select the wrong component. */ + for (p = location; (p = strstr(p, "->")) != NULL; p += 2) { + arrow = p; + } + + mod = arrow ? arrow + 2 : location; + if (strcmp(mod, SYSCHECK) == 0 || strcmp(mod, SYSCHECK_REG) == 0) { + return (1); + } + + return (0); +} + void remove_control_characters_ex(char *str, int preserve_nl) { - int len = strlen(str); + int len; int i, j; + if (!str) { + return; + } + + len = (int)strlen(str); + for (i = 0, j = 0; i < len; i++) { char c = str[i];