diff --git a/booth.spec b/booth.spec index ec505c572e6e7193affadcdf30cd5b9fa66be50e..0c6f08417949ed86a58c0369a0874dbb31d9e8a0 100644 --- a/booth.spec +++ b/booth.spec @@ -24,7 +24,7 @@ %bcond_with run_build_tests %bcond_with include_unit_test -%global release 4 +%global release 5 ## User and group to use for nonprivileged services (should be in sync with pacemaker) %global uname hacluster @@ -51,6 +51,7 @@ Source0: https://github.com/%{github_owner}/%{name}/releases/download/v%{ Patch0: Remove-const-warning.patch Patch1: pacemaker-Don-t-add-explicit-error-prefix-in-log.patch Patch2: pacemaker-Use-long-format-for-crm_ticket-v.patch +Patch3: pacemaker-Check-snprintf-return-values.patch # direct build process dependencies BuildRequires: autoconf @@ -299,6 +300,9 @@ VERBOSE=1 make check %{_usr}/lib/ocf/resource.d/booth/sharedrsc %changelog +* Sun Apr 28 2024 bizhiyuan - 1.1-5 +- pacemaker Check snprintf return values + * Thu Feb 29 2024 bizhiyuan - 1.1-4 - pacemaker: Use long format for crm_ticket -v diff --git a/pacemaker-Check-snprintf-return-values.patch b/pacemaker-Check-snprintf-return-values.patch new file mode 100644 index 0000000000000000000000000000000000000000..312a010a37ca606997e096a1e55b5447c631eef5 --- /dev/null +++ b/pacemaker-Check-snprintf-return-values.patch @@ -0,0 +1,124 @@ +From 7e33a45d6898e06119dbe9dfd487f6c4923b48cb Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Tue, 14 Nov 2023 17:21:49 +0100 +Subject: [PATCH 2/7] pacemaker: Check snprintf return values + +crm_ticket command string is stored into static buffer and not checked +so it can be truncated without notice. + +Solution would be to use dynamic buffer, but for now at least check +snprintf return value and return error when string was truncated. + +Signed-off-by: Jan Friesse +--- + src/pacemaker.c | 39 ++++++++++++++++++++++++++++++++++----- + 1 file changed, 34 insertions(+), 5 deletions(-) + +diff --git a/src/pacemaker.c b/src/pacemaker.c +index 8ad3c69..80aa1a3 100644 +--- a/src/pacemaker.c ++++ b/src/pacemaker.c +@@ -128,7 +128,7 @@ static int pcmk_write_ticket_atomic(struct ticket_config *tk, int grant) + + /* The long format (--attr-value=) for attribute value is used instead of "-v", + * so that NO_ONE (which is -1) isn't seen as another option. */ +- snprintf(cmd, COMMAND_MAX, ++ rv = snprintf(cmd, COMMAND_MAX, + "crm_ticket -t '%s' " + "%s --force " + "-S owner --attr-value=%" PRIi32 " " +@@ -142,6 +142,10 @@ static int pcmk_write_ticket_atomic(struct ticket_config *tk, int grant) + (int64_t)wall_ts(&tk->term_expires), + (int64_t)tk->current_term); + ++ if (rv < 0 || rv >= COMMAND_MAX) { ++ log_error("pcmk_write_ticket_atomic: cannot format crm_ticket cmdline (probably too long)"); ++ return -1; ++ } + rv = system(cmd); + log_debug("command: '%s' was executed", cmd); + if (rv != 0) +@@ -230,20 +234,34 @@ static int crm_ticket_set_int(const struct ticket_config *tk, const char *attr, + static int pcmk_set_attr(struct ticket_config *tk, const char *attr, const char *val) + { + char cmd[COMMAND_MAX]; ++ int rv; + +- snprintf(cmd, COMMAND_MAX, ++ rv = snprintf(cmd, COMMAND_MAX, + "crm_ticket -t '%s' -S '%s' --attr-value='%s'", + tk->name, attr, val); ++ ++ if (rv < 0 || rv >= COMMAND_MAX) { ++ log_error("pcmk_set_attr: cannot format crm_ticket cmdline (probably too long)"); ++ return -1; ++ } ++ + return _run_crm_ticket(cmd); + } + + static int pcmk_del_attr(struct ticket_config *tk, const char *attr) + { + char cmd[COMMAND_MAX]; ++ int rv; + +- snprintf(cmd, COMMAND_MAX, ++ rv = snprintf(cmd, COMMAND_MAX, + "crm_ticket -t '%s' -D '%s'", + tk->name, attr); ++ ++ if (rv < 0 || rv >= COMMAND_MAX) { ++ log_error("pcmk_del_attr: cannot format crm_ticket cmdline (probably too long)"); ++ return -1; ++ } ++ + return _run_crm_ticket(cmd); + } + +@@ -352,13 +370,18 @@ static int pcmk_get_attr(struct ticket_config *tk, const char *attr, const char + char cmd[COMMAND_MAX]; + char line[BOOTH_ATTRVAL_LEN+1]; + int rv = 0, pipe_rv; ++ int res; + FILE *p; + + + *vp = NULL; +- snprintf(cmd, COMMAND_MAX, ++ res = snprintf(cmd, COMMAND_MAX, + "crm_ticket -t '%s' -G '%s' --quiet", + tk->name, attr); ++ if (res < 0 || res >= COMMAND_MAX) { ++ log_error("pcmk_get_attr: cannot format crm_ticket cmdline (probably too long)"); ++ return -1; ++ } + + p = popen(cmd, "r"); + if (p == NULL) { +@@ -483,16 +506,22 @@ static int pcmk_load_ticket(struct ticket_config *tk) + { + char cmd[COMMAND_MAX]; + int rv = 0, pipe_rv; ++ int res; + FILE *p; + + /* This here gets run during startup; testing that here means that + * normal operation won't be interrupted with that test. */ + test_atomicity(); + +- snprintf(cmd, COMMAND_MAX, ++ res = snprintf(cmd, COMMAND_MAX, + "crm_ticket -t '%s' -q", + tk->name); + ++ if (res < 0 || res >= COMMAND_MAX) { ++ log_error("pcmk_load_ticket: cannot format crm_ticket cmdline (probably too long)"); ++ return -1; ++ } ++ + p = popen(cmd, "r"); + if (p == NULL) { + pipe_rv = errno; +-- +2.33.0 +