From 571fba729e33b3c43eafff1af4965d7615bbfd2e Mon Sep 17 00:00:00 2001 From: Ben Kibbey Date: Thu, 14 Jun 2012 22:21:41 -0400 Subject: [PATCH] Add validation of "target" attributes. Try to prevent recursion loops when creating a "target" attribute for an element. This is easier than preventing LIST getting stuck in a loop after creating a target, but isn't foolproof since an element may be DELETE'd causing a loop to occur. It is only a temporary fix but should be Good Enough for most users. --- src/commands.c | 10 ++- src/xml.c | 221 +++++++++++++++++++++++++++++++++++++++++---------------- src/xml.h | 2 + 3 files changed, 165 insertions(+), 68 deletions(-) diff --git a/src/commands.c b/src/commands.c index a4322a9f..3d12e9ed 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1602,14 +1602,12 @@ static gpg_error_t target_attribute(struct client_s *client, gchar **req) } odst = g_strdupv(dst); - if (!odst) { rc = GPG_ERR_ENOMEM; goto fail; } n = find_root_element(client->doc, &dst, &rc, NULL, 0, FALSE); - /* * Make sure the destination element path exists. */ @@ -1619,21 +1617,21 @@ static gpg_error_t target_attribute(struct client_s *client, gchar **req) if (dst[1]) { n = find_elements(client->doc, n->children, dst+1, &rc, NULL, NULL, NULL, FALSE, 0, NULL, FALSE); - if (!n) goto fail; } - n = create_element_path(client, &src, &rc, NULL); + rc = validate_target_attribute(client->doc, req[0], n); + if (rc) + goto fail; + n = create_element_path(client, &src, &rc, NULL); if (rc) goto fail; line = g_strjoinv("\t", odst); - if (!line) { rc = GPG_ERR_ENOMEM; - log_write("%s(%i): %s", __FILE__, __LINE__, pwmd_strerror(GPG_ERR_ENOMEM)); goto fail; } diff --git a/src/xml.c b/src/xml.c index 75a2e3ce..71e1a235 100644 --- a/src/xml.c +++ b/src/xml.c @@ -883,63 +883,6 @@ static gboolean update_element_list(struct element_list_s *elements) return TRUE; } -/* Search the tree beginning at "start" for children whose xmlNodePtr match - * "root". Needed to prevent recursion loops during LIST. */ -static gpg_error_t find_children_of_target(xmlDocPtr doc, xmlNodePtr root, - xmlNodePtr start, gchar **start_req) -{ - xmlNodePtr n; - gpg_error_t rc = 0; - - for (n = start; n; n = n->next) { - gchar **req_copy, **new_req = NULL; - xmlNodePtr node; - xmlChar *path; - - if (n->type != XML_ELEMENT_NODE) - continue; - - xmlChar *a = node_has_attribute(n, (xmlChar *)"_name"); - if (!a) - continue; - - req_copy = g_strdupv(start_req); - if (!req_copy) - return GPG_ERR_ENOMEM; - - if (!strv_printf(&req_copy, "%s", a)) { - xmlFree(a); - return GPG_ERR_ENOMEM; - } - - xmlFree(a); - path = (xmlChar *)g_strjoinv("\t", req_copy); - g_strfreev(req_copy); - if (!path) - return GPG_ERR_ENOMEM; - - node = resolve_path(doc, path, &new_req, &rc); - g_free(path); - if (!rc) { - if (node == root || node->children == root) - rc = GPG_ERR_ELOOP; - } - - if (rc) { - if (new_req) - g_strfreev(new_req); - break; - } - - rc = find_children_of_target(doc, root, node->children, new_req); - g_strfreev(new_req); - if (rc) - break; - } - - return rc; -} - static gpg_error_t path_list_recurse(xmlDocPtr doc, xmlNodePtr node, struct element_list_s *elements) { @@ -984,11 +927,6 @@ static gpg_error_t path_list_recurse(xmlDocPtr doc, xmlNodePtr node, GString *realpath = NULL; tnode = resolve_path(doc, target, &req, &rc); - if (!rc) { - rc = find_children_of_target(doc, node, tnode->children, req); - g_strfreev(req); - } - if (rc == GPG_ERR_ELOOP || rc == GPG_ERR_ELEMENT_NOT_FOUND) { if (rc == GPG_ERR_ELOOP) { xmlChar *t = xmlGetNodePath(n); @@ -1605,3 +1543,162 @@ again: *result = string; return rc; } + +#if 0 +static gchar *node_to_element_path(xmlNodePtr node) +{ + xmlNodePtr n; + GString *str = g_string_new(""); + gchar *result; + + for (n = node; n; n = n->parent) { + xmlNodePtr child; + + for (child = n; child; child = child->next) { + if (child->type != XML_ELEMENT_NODE) + continue; + + xmlChar *name = node_has_attribute(n, (xmlChar *)"_name"); + if (name) { + str = g_string_prepend(str, (gchar *)name); + xmlFree(name); + name = node_has_attribute(n, (xmlChar *)"target"); + if (name) + str = g_string_prepend(str, "\t"); + else + str = g_string_prepend(str, "\t!"); + xmlFree(name); + } + break; + } + } + + str = g_string_erase(str, 0, 1); + result = str->str; + g_string_free(str, FALSE); + return result; +} +#endif + +/* + * Recurse the element tree beginning at 'node' and find elements who point + * back to 'src' or 'dst'. Also follows target attributes. + */ +static gpg_error_t find_child_to_target(xmlDocPtr doc, xmlNodePtr node, + xmlNodePtr src, xmlNodePtr dst) +{ + xmlNodePtr n; + gpg_error_t rc = 0; + + for (n = node; n; n = n->next) { + xmlChar *target; + + if (n->type != XML_ELEMENT_NODE) + continue; + + if (n == src || n == dst) + return GPG_ERR_ELOOP; + + target = node_has_attribute(n, (xmlChar *)"target"); + if (target) { + xmlNodePtr tmp; + gchar **result = NULL; + + tmp = resolve_path(doc, target, &result, &rc); + xmlFree(target); + g_strfreev(result); + if (!rc) { + rc = find_child_to_target(doc, tmp, src, dst); + if (rc) + return rc; + } + } + else + xmlFree(target); + + rc = find_child_to_target(doc, n->children, src, dst); + if (rc) + return rc; + } + + return rc; +} + +static gpg_error_t find_child_of_parent(xmlDocPtr doc, xmlNodePtr src, + xmlNodePtr dst) +{ + xmlNodePtr n; + gpg_error_t rc = 0; + + for (n = src; n; n = n->next) { + if (n->type != XML_ELEMENT_NODE) + continue; + + if (n == dst) { + rc = GPG_ERR_ELOOP; + break; + } + + rc = find_child_of_parent(doc, src->children, dst); + } + + return rc; +} + +static gpg_error_t find_parent_of_child(xmlDocPtr doc, xmlNodePtr node, + xmlNodePtr dst) +{ + xmlNodePtr n; + gpg_error_t rc = 0; + + for (n = node; n; n = n->parent) { + if (n->type != XML_ELEMENT_NODE) { + xmlNodePtr tmp; + + for (tmp = n->next; tmp; n = n->next) { + if (n->type != XML_ELEMENT_NODE) + continue; + + if (tmp == dst) + return GPG_ERR_ELOOP; + } + } + + if (n == dst) + return GPG_ERR_ELOOP; + } + + return rc; +} + +gpg_error_t validate_target_attribute(xmlDocPtr doc, const gchar *src, + xmlNodePtr dst_node) +{ + gpg_error_t rc; + xmlNodePtr src_node; + gchar **src_req = NULL; + + /* If the source does not exists then it will be created. */ + src_node = resolve_path(doc, (xmlChar *)src, &src_req, &rc); + if (rc && rc == GPG_ERR_ELEMENT_NOT_FOUND) + return 0; + + /* A destination element is a child of the source element. */ + rc = find_child_of_parent(doc, src_node->children, dst_node); + if (rc) + goto fail; + + /* The destination element is a parent of the source element. */ + rc = find_parent_of_child(doc, src_node->parent, dst_node); + if (rc) + goto fail; + + /* A destination child element contains a target to the source element. */ + rc = find_child_to_target(doc, dst_node->children, src_node, dst_node); + if (rc) + goto fail; + +fail: + g_strfreev(src_req); + return rc; +} diff --git a/src/xml.h b/src/xml.h index 7ba2ef78..5931a1c1 100644 --- a/src/xml.h +++ b/src/xml.h @@ -72,5 +72,7 @@ gpg_error_t unlink_node(xmlNodePtr n); xmlDocPtr parse_doc(const gchar *xml, gsize len); gpg_error_t attr_ctime(xmlNodePtr n); gpg_error_t build_realpath(xmlDocPtr doc, gchar *line, GString **result); +gpg_error_t validate_target_attribute(xmlDocPtr doc, const gchar *src, + xmlNodePtr dst); #endif -- 2.11.4.GIT