pacemaker Check snprintf return values

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 <bizhiyuan@kylinos.cn> - 1.1-5
+- pacemaker Check snprintf return values
+
 * Thu Feb 29 2024 bizhiyuan <bizhiyuan@kylinos.cn> - 1.1-4
 - pacemaker: Use long format for crm_ticket -v
 

pacemaker-Check-snprintf-return-values.patch

@@ -0,0 +1,124 @@
+From 7e33a45d6898e06119dbe9dfd487f6c4923b48cb Mon Sep 17 00:00:00 2001
+From: Jan Friesse <jfriesse@redhat.com>
+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 <jfriesse@redhat.com>
+---
+ 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
+