From adce225a104d0b7503aa7322db15d1c6dd8b8093 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Dec 2020 04:36:14 +0900 Subject: [PATCH 1/3] journal: move journal_field_valid() to journal_file.c --- src/core/dbus-execute.c | 2 +- src/core/load-fragment.c | 2 +- src/journal/journal-file.c | 38 ++++++++++++++++++++++++++++++++++ src/journal/journal-file.h | 2 ++ src/shared/journal-util.c | 38 ---------------------------------- src/shared/journal-util.h | 1 - src/systemctl/systemctl-show.c | 2 +- 7 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index abe009c395..04735354a3 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -24,7 +24,7 @@ #include "hexdecoct.h" #include "io-util.h" #include "ioprio.h" -#include "journal-util.h" +#include "journal-file.h" #include "mountpoint-util.h" #include "namespace.h" #include "parse-util.h" diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index ffc58dde9c..4964249bf2 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -38,7 +38,7 @@ #include "io-util.h" #include "ioprio.h" #include "ip-protocol-list.h" -#include "journal-util.h" +#include "journal-file.h" #include "limits-util.h" #include "load-fragment.h" #include "log.h" diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 20c4edb6ca..18dc3072b4 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -1521,6 +1521,44 @@ int journal_file_find_data_object( ret, ret_offset); } +bool journal_field_valid(const char *p, size_t l, bool allow_protected) { + const char *a; + + /* We kinda enforce POSIX syntax recommendations for + environment variables here, but make a couple of additional + requirements. + + http://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html */ + + if (l == (size_t) -1) + l = strlen(p); + + /* No empty field names */ + if (l <= 0) + return false; + + /* Don't allow names longer than 64 chars */ + if (l > 64) + return false; + + /* Variables starting with an underscore are protected */ + if (!allow_protected && p[0] == '_') + return false; + + /* Don't allow digits as first character */ + if (p[0] >= '0' && p[0] <= '9') + return false; + + /* Only allow A-Z0-9 and '_' */ + for (a = p; a < p + l; a++) + if ((*a < 'A' || *a > 'Z') && + (*a < '0' || *a > '9') && + *a != '_') + return false; + + return true; +} + static int journal_file_append_field( JournalFile *f, const void *field, uint64_t size, diff --git a/src/journal/journal-file.h b/src/journal/journal-file.h index 263d032b90..931c874268 100644 --- a/src/journal/journal-file.h +++ b/src/journal/journal-file.h @@ -271,3 +271,5 @@ static inline bool JOURNAL_FILE_COMPRESS(JournalFile *f) { } uint64_t journal_file_hash_data(JournalFile *f, const void *data, size_t sz); + +bool journal_field_valid(const char *p, size_t l, bool allow_protected); diff --git a/src/shared/journal-util.c b/src/shared/journal-util.c index 29659aa6b7..9e1870e176 100644 --- a/src/shared/journal-util.c +++ b/src/shared/journal-util.c @@ -137,41 +137,3 @@ int journal_access_check_and_warn(sd_journal *j, bool quiet, bool want_other_use return r; } - -bool journal_field_valid(const char *p, size_t l, bool allow_protected) { - const char *a; - - /* We kinda enforce POSIX syntax recommendations for - environment variables here, but make a couple of additional - requirements. - - http://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html */ - - if (l == (size_t) -1) - l = strlen(p); - - /* No empty field names */ - if (l <= 0) - return false; - - /* Don't allow names longer than 64 chars */ - if (l > 64) - return false; - - /* Variables starting with an underscore are protected */ - if (!allow_protected && p[0] == '_') - return false; - - /* Don't allow digits as first character */ - if (p[0] >= '0' && p[0] <= '9') - return false; - - /* Only allow A-Z0-9 and '_' */ - for (a = p; a < p + l; a++) - if ((*a < 'A' || *a > 'Z') && - (*a < '0' || *a > '9') && - *a != '_') - return false; - - return true; -} diff --git a/src/shared/journal-util.h b/src/shared/journal-util.h index db7000ffef..86fcba058d 100644 --- a/src/shared/journal-util.h +++ b/src/shared/journal-util.h @@ -6,6 +6,5 @@ #include "sd-journal.h" -bool journal_field_valid(const char *p, size_t l, bool allow_protected); int journal_access_blocked(sd_journal *j); int journal_access_check_and_warn(sd_journal *j, bool quiet, bool want_other_users); diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index fabaa545e1..d5efecbe65 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -16,7 +16,7 @@ #include "hexdecoct.h" #include "hostname-util.h" #include "in-addr-util.h" -#include "journal-util.h" +#include "journal-file.h" #include "list.h" #include "locale-util.h" #include "memory-util.h" From f2bd032044ca3cd4b454dd0ba86719effcf34dc0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Dec 2020 04:44:31 +0900 Subject: [PATCH 2/3] journal: refuse data which contain invalid fields Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=25353. --- src/journal/journal-file.c | 3 +++ test/fuzz/fuzz-journal-remote/oss-fuzz-25353 | Bin 0 -> 45 bytes 2 files changed, 3 insertions(+) create mode 100644 test/fuzz/fuzz-journal-remote/oss-fuzz-25353 diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 18dc3072b4..fa117ab63c 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -1572,6 +1572,9 @@ static int journal_file_append_field( assert(f); assert(field && size > 0); + if (!journal_field_valid(field, size, true)) + return -EBADMSG; + hash = journal_file_hash_data(f, field, size); r = journal_file_find_field_object_with_hash(f, field, size, hash, &o, &p); diff --git a/test/fuzz/fuzz-journal-remote/oss-fuzz-25353 b/test/fuzz/fuzz-journal-remote/oss-fuzz-25353 new file mode 100644 index 0000000000000000000000000000000000000000..94e5fbb93e6badfa33b420d12b9c92528fc9ac92 GIT binary patch literal 45 lcma!#4{~+%3GwuGjRz6IA&$NQwuW5)xwsf0KmiW8xBv&T2?PKD literal 0 HcmV?d00001 From 805d67c565d57e0915162164f7e5e3026a29a2c5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Dec 2020 04:50:39 +0900 Subject: [PATCH 3/3] logs-show: refuse data which contain invalid fields --- src/shared/logs-show.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index bf574d32a5..840f221fff 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -702,9 +702,11 @@ static int output_verbose( c = memchr(data, '=', length); if (!c) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid field."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid field."); + fieldlen = c - (const char*) data; + if (!journal_field_valid(data, fieldlen, true)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid field."); r = field_set_test(output_fields, data, fieldlen); if (r < 0) @@ -798,6 +800,7 @@ static int output_export( sd_id128_to_string(boot_id, sid)); JOURNAL_FOREACH_DATA_RETVAL(j, data, length, r) { + size_t fieldlen; const char *c; /* We already printed the boot id from the data in the header, hence let's suppress it here */ @@ -806,10 +809,13 @@ static int output_export( c = memchr(data, '=', length); if (!c) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid field."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid field."); - r = field_set_test(output_fields, data, c - (const char *) data); + fieldlen = c - (const char*) data; + if (!journal_field_valid(data, fieldlen, true)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid field."); + + r = field_set_test(output_fields, data, fieldlen); if (r < 0) return r; if (!r) @@ -820,11 +826,11 @@ static int output_export( else { uint64_t le64; - fwrite(data, c - (const char*) data, 1, f); + fwrite(data, fieldlen, 1, f); fputc('\n', f); - le64 = htole64(length - (c - (const char*) data) - 1); + le64 = htole64(length - fieldlen - 1); fwrite(&le64, sizeof(le64), 1, f); - fwrite(c + 1, length - (c - (const char*) data) - 1, 1, f); + fwrite(c + 1, length - fieldlen - 1, 1, f); } fputc('\n', f); @@ -961,6 +967,7 @@ static int update_json_data_split( const void *data, size_t size) { + size_t fieldlen; const char *eq; char *name; @@ -974,14 +981,15 @@ static int update_json_data_split( if (!eq) return 0; - if (eq == data) - return 0; + fieldlen = eq - (const char*) data; + if (!journal_field_valid(data, fieldlen, true)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid field."); - name = strndupa(data, eq - (const char*) data); + name = strndupa(data, fieldlen); if (output_fields && !set_contains(output_fields, name)) return 0; - return update_json_data(h, flags, name, eq + 1, size - (eq - (const char*) data) - 1); + return update_json_data(h, flags, name, eq + 1, size - fieldlen - 1); } static int output_json(