summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorGravatar Nikias Bassen2019-08-09 19:59:05 +0200
committerGravatar Nikias Bassen2019-08-09 19:59:05 +0200
commite1a5d60e98b72fe110391da848c77cc36665bd66 (patch)
tree9b3f2bd9960aa0a2e17c8c6d53e53646d7638eec /src
parent811a53aefe4693113ef723783c151e473853a398 (diff)
downloadlibplist-e1a5d60e98b72fe110391da848c77cc36665bd66.tar.gz
libplist-e1a5d60e98b72fe110391da848c77cc36665bd66.tar.bz2
Make sure to copy hash table entries properly when cloning array/dict nodes
As mentioned in #142, plist_copy_node() was not correctly handling the hash tables when cloning array or dict nodes; it incorrectly filled the hash table with the original child node info, which effectively would lead to a segmentation fault / UaF if the original array/dict would be freed followed by an attempt to access an element in the new hash table.
Diffstat (limited to 'src')
-rw-r--r--src/plist.c40
1 files changed, 22 insertions, 18 deletions
diff --git a/src/plist.c b/src/plist.c
index 6d7a07f..4600198 100644
--- a/src/plist.c
+++ b/src/plist.c
@@ -321,7 +321,7 @@ PLIST_API void plist_free(plist_t plist)
}
}
-static void plist_copy_node(node_t *node, void *parent_node_ptr)
+static plist_t plist_copy_node(node_t *node, void *parent_node_ptr)
{
plist_type node_type = PLIST_NONE;
plist_t newnode = NULL;
@@ -347,13 +347,6 @@ static void plist_copy_node(node_t *node, void *parent_node_ptr)
if (data->hashtable) {
ptrarray_t* pa = ptr_array_new(((ptrarray_t*)data->hashtable)->capacity);
assert(pa);
- plist_t current = NULL;
- for (current = (plist_t)node_first_child(node);
- pa && current;
- current = (plist_t)node_next_sibling(current))
- {
- ptr_array_add(pa, current);
- }
newdata->hashtable = pa;
}
break;
@@ -361,13 +354,6 @@ static void plist_copy_node(node_t *node, void *parent_node_ptr)
if (data->hashtable) {
hashtable_t* ht = hash_table_new(dict_key_hash, dict_key_compare, NULL);
assert(ht);
- plist_t current = NULL;
- for (current = (plist_t)node_first_child(node);
- ht && current;
- current = (plist_t)node_next_sibling(node_next_sibling(current)))
- {
- hash_table_insert(ht, ((node_t*)current)->data, node_next_sibling(current));
- }
newdata->hashtable = ht;
}
break;
@@ -386,16 +372,34 @@ static void plist_copy_node(node_t *node, void *parent_node_ptr)
}
node_t *ch;
+ unsigned int node_index = 0;
for (ch = node_first_child(node); ch; ch = node_next_sibling(ch)) {
- plist_copy_node(ch, &newnode);
+ /* copy child node and attach to new parent node */
+ plist_t newch = plist_copy_node(ch, &newnode);
+ /* if needed, add child node to lookup table of parent node */
+ switch (node_type) {
+ case PLIST_ARRAY:
+ if (newdata->hashtable) {
+ ptr_array_add((ptrarray_t*)newdata->hashtable, newch);
+ }
+ break;
+ case PLIST_DICT:
+ if (newdata->hashtable && (node_index % 2 != 0)) {
+ hash_table_insert((hashtable_t*)newdata->hashtable, (node_prev_sibling((node_t*)newch))->data, newch);
+ }
+ break;
+ default:
+ break;
+ }
+ node_index++;
}
+ return newnode;
}
PLIST_API plist_t plist_copy(plist_t node)
{
plist_t copied = NULL;
- plist_copy_node(node, &copied);
- return copied;
+ return plist_copy_node(node, &copied);
}
PLIST_API uint32_t plist_array_get_size(plist_t node)