File pacemaker-attrd-validate-request.patch of Package pacemaker.3577
commit 755fb18bf286852bdf7205ea9bbd2641fa151bc7
Author: Ken Gaillot <kgaillot@redhat.com>
Date: Tue May 24 15:23:52 2016 -0500
Fix: attrd, libcrmcommon: validate attrd requests better
This prevents potential attrd segfaults due to malformed client requests.
attrd_update_delegate() may now return -EINVAL if a required argument
is not supplied.
Fixes CLBZ#5272
Index: pacemaker/attrd/commands.c
===================================================================
--- pacemaker.orig/attrd/commands.c
+++ pacemaker/attrd/commands.c
@@ -249,6 +249,10 @@ attrd_client_update(xmlNode *xml)
regfree(r_patt);
free(r_patt);
return;
+
+ } else if (attr == NULL) {
+ crm_err("Update request did not specify attribute or regular expression");
+ return;
}
if (host == NULL) {
@@ -668,13 +672,18 @@ void
attrd_peer_update(crm_node_t *peer, xmlNode *xml, const char *host, bool filter)
{
bool changed = FALSE;
+ attribute_t *a;
attribute_value_t *v = NULL;
const char *attr = crm_element_value(xml, F_ATTRD_ATTRIBUTE);
const char *value = crm_element_value(xml, F_ATTRD_VALUE);
- attribute_t *a = g_hash_table_lookup(attributes, attr);
+ if (attr == NULL) {
+ crm_warn("Peer update did not specify attribute");
+ return;
+ }
+ a = g_hash_table_lookup(attributes, attr);
if(a == NULL) {
a = create_attribute(xml);
}
Index: pacemaker/lib/common/utils.c
===================================================================
--- pacemaker.orig/lib/common/utils.c
+++ pacemaker/lib/common/utils.c
@@ -1735,6 +1735,8 @@ attrd_update_delegate(crm_ipc_t * ipc, c
{
int rc = -ENOTCONN;
int max = 5;
+ const char *task = NULL;
+ const char *name_as = NULL;
xmlNode *update = create_xml_node(NULL, __FUNCTION__);
static gboolean connected = TRUE;
@@ -1768,27 +1770,36 @@ attrd_update_delegate(crm_ipc_t * ipc, c
switch (command) {
case 'u':
- crm_xml_add(update, F_ATTRD_TASK, ATTRD_OP_UPDATE);
- crm_xml_add(update, F_ATTRD_REGEX, name);
+ task = ATTRD_OP_UPDATE;
+ name_as = F_ATTRD_REGEX;
break;
case 'D':
case 'U':
case 'v':
- crm_xml_add(update, F_ATTRD_TASK, ATTRD_OP_UPDATE);
- crm_xml_add(update, F_ATTRD_ATTRIBUTE, name);
+ task = ATTRD_OP_UPDATE;
+ name_as = F_ATTRD_ATTRIBUTE;
break;
case 'R':
- crm_xml_add(update, F_ATTRD_TASK, ATTRD_OP_REFRESH);
+ task = ATTRD_OP_REFRESH;
break;
case 'Q':
- crm_xml_add(update, F_ATTRD_TASK, ATTRD_OP_QUERY);
- crm_xml_add(update, F_ATTRD_ATTRIBUTE, name);
+ task = ATTRD_OP_QUERY;
+ name_as = F_ATTRD_ATTRIBUTE;
break;
case 'C':
- crm_xml_add(update, F_ATTRD_TASK, ATTRD_OP_PEER_REMOVE);
+ task = ATTRD_OP_PEER_REMOVE;
break;
}
+ if (name_as != NULL) {
+ if (name == NULL) {
+ rc = -EINVAL;
+ goto done;
+ }
+ crm_xml_add(update, name_as, name);
+ }
+
+ crm_xml_add(update, F_ATTRD_TASK, task);
crm_xml_add(update, F_ATTRD_VALUE, value);
crm_xml_add(update, F_ATTRD_DAMPEN, dampen);
crm_xml_add(update, F_ATTRD_SECTION, section);
@@ -1832,6 +1843,7 @@ attrd_update_delegate(crm_ipc_t * ipc, c
}
}
+done:
free_xml(update);
if (rc > 0) {
crm_debug("Sent update: %s=%s for %s", name, value, host ? host : "localhost");