xplist: Enforce single root value inside <plist>

Ensure that XML property lists contain exactly one root value inside the <plist> element and reject any additional value nodes before </plist>.

Add tests covering root value handling and nested CF$UID conversion behavior.

Co-authored-by: Sami Kortelainen <sami.kortelainen@piceasoft.com>
Co-authored-by: Nikias Bassen <nikias@gmx.li>

5 files changed, 203 insertions, 17 deletions

diff --git a/.gitignore b/.gitignore
index e63b0b6..a59b057 100644
--- a/.gitignore
+++ b/.gitignore
@@ -54,6 +54,7 @@ test/plist_btest
 test/plist_jtest
 test/plist_otest
 test/integer_set_test
+test/xml_behavior_test
 test/data/*.out
 test/*.trs
 cython/Makefile
@@ -62,3 +63,12 @@ cython/.deps
 cython/.libs
 cython/plist.c
 test-driver
+
+# Generated test output files
+test/data/*.test.bin
+test/data/*.test.signed.bin
+test/data/*.test.unsigned.bin
+test/data/*.test.unsigned.xml
+test/data/*.test.tz*.bin
+test/data/*.test.tz*.xml
+test/data/*.test.xml
diff --git a/src/xplist.c b/src/xplist.c
index a445dc5..de5227a 100644
--- a/src/xplist.c
+++ b/src/xplist.c
@@ -1170,8 +1170,9 @@ static plist_err_t node_from_xml(parse_ctx ctx, plist_t *plist)
             ctx->pos++;
             if (!strcmp(tag, "plist")) {
                 if (!node_path && *plist) {
-                    /* we don't allow another top-level <plist> */
-                    break;
+                    PLIST_XML_ERR("Multiple top-level <plist> elements encountered\n");
+                    ctx->err = PLIST_ERR_PARSE;
+                    goto err_out;
                 }
                 if (is_empty) {
                     PLIST_XML_ERR("Empty plist tag\n");
@@ -1403,12 +1404,6 @@ static plist_err_t node_from_xml(parse_ctx ctx, plist_t *plist)
                         data->length = length;
                     }
                 } else {
-                    if (!strcmp(tag, "key") && !keyname && parent && (plist_get_node_type(parent) == PLIST_DICT)) {
-                        keyname = strdup("");
-                        plist_free(subnode);
-                        subnode = NULL;
-                        continue;
-                    }
                     data->strval = strdup("");
                     data->length = 0;
                 }
@@ -1501,14 +1496,15 @@ static plist_err_t node_from_xml(parse_ctx ctx, plist_t *plist)
             }
             if (subnode && !closing_tag) {
                 if (!*plist) {
-                    /* first node, make this node the parent node */
+                    /* first value node inside <plist> */
                     *plist = subnode;
-                    if (data->type != PLIST_DICT && data->type != PLIST_ARRAY) {
-                        /* if the first node is not a structered node, we're done */
-                        subnode = NULL;
-                        goto err_out;
+
+                    if (data->type == PLIST_DICT || data->type == PLIST_ARRAY) {
+                        parent = subnode;
+                    } else {
+                        /* scalar root: keep parsing until </plist> */
+                        parent = NULL;
                     }
-                    parent = subnode;
                 } else if (parent) {
                     switch (plist_get_node_type(parent)) {
                     case PLIST_DICT:
@@ -1528,6 +1524,11 @@ static plist_err_t node_from_xml(parse_ctx ctx, plist_t *plist)
                         ctx->err = PLIST_ERR_PARSE;
                         goto err_out;
                     }
+                } else {
+                    /* We already produced root, and we're not inside a container */
+                    PLIST_XML_ERR("Unexpected tag <%s> found while </plist> is expected\n", tag);
+                    ctx->err = PLIST_ERR_PARSE;
+                    goto err_out;
                 }
                 if (!is_empty && (data->type == PLIST_DICT || data->type == PLIST_ARRAY)) {
                     if (depth >= PLIST_MAX_NESTING_DEPTH) {
@@ -1547,6 +1548,8 @@ static plist_err_t node_from_xml(parse_ctx ctx, plist_t *plist)
 
                     depth++;
                     parent = subnode;
+                } else {
+                    /* If we inserted a child scalar into a container, nothing to push. */
                 }
                 subnode = NULL;
             }
@@ -1587,7 +1590,6 @@ handle_closing:
                 node_path = (struct node_path_item*)node_path->prev;
                 free(path_item);
                 parent = (parent) ? ((node_t)parent)->parent : NULL;
-                /* parent can be NULL when we just closed the root node; keep parsing */
             }
             free(keyname);
             keyname = NULL;
diff --git a/test/Makefile.am b/test/Makefile.am
index a4191c4..f9f21e4 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -13,7 +13,8 @@ noinst_PROGRAMS = \
         integer_set_test \
         plist_btest \
         plist_jtest \
-        plist_otest
+        plist_otest \
+        xml_behavior_test
 
 plist_cmp_SOURCES = plist_cmp.c
 plist_cmp_LDADD = \
@@ -38,6 +39,9 @@ plist_jtest_LDADD = $(top_builddir)/src/libplist-2.0.la
 plist_otest_SOURCES = plist_otest.c
 plist_otest_LDADD = $(top_builddir)/src/libplist-2.0.la
 
+xml_behavior_test_SOURCES = xml_behavior_test.c
+xml_behavior_test_LDADD = $(top_builddir)/src/libplist-2.0.la
+
 TESTS = \
         empty.test \
         small.test \
@@ -79,7 +83,8 @@ TESTS = \
         ostep2.test \
         ostep-strings.test \
         ostep-comments.test \
-        ostep-invalid-types.test
+        ostep-invalid-types.test \
+        xml_behavior.test
 
 EXTRA_DIST = \
         $(TESTS) \
diff --git a/test/xml_behavior.test b/test/xml_behavior.test
new file mode 100755
index 0000000..81d8dd0
--- /dev/null
+++ b/test/xml_behavior.test
@@ -0,0 +1,2 @@
+## -*- sh -*-
+$top_builddir/test/xml_behavior_test
diff --git a/test/xml_behavior_test.c b/test/xml_behavior_test.c
new file mode 100644
index 0000000..94d8a7f
--- /dev/null
+++ b/test/xml_behavior_test.c
@@ -0,0 +1,167 @@
+/*
+ * xml_behavior_test.c
+ *
+ * Tests XML parser behavior for correctness and specification compliance:
+ *
+ * 1) A <plist> element must contain exactly one root value node.
+ *    Any additional value nodes after the first root object must
+ *    cause parsing to fail.
+ *
+ * 2) Dictionaries of the form:
+ *        <dict>
+ *          <key>CF$UID</key>
+ *          <integer>...</integer>
+ *        </dict>
+ *    must be converted to PLIST_UID nodes during XML parsing,
+ *    including when they appear nested inside other containers.
+ *
+ * These tests ensure proper root handling and UID node conversion
+ * when parsing XML property lists.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
+
+#include "plist/plist.h"
+
+static int test_nested_cfuid_converts_to_uid(void)
+{
+    const char *xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+        "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" "
+        "\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">"
+        "<plist version=\"1.0\">"
+        "  <dict>"
+        "    <key>obj</key>"
+        "    <dict>"
+        "      <key>CF$UID</key>"
+        "      <integer>7</integer>"
+        "    </dict>"
+        "  </dict>"
+        "</plist>";
+
+    plist_t root = NULL;
+    plist_err_t err = plist_from_xml(xml, (uint32_t)strlen(xml), &root);
+    if (err != PLIST_ERR_SUCCESS || !root) {
+        fprintf(stderr, "nested CF$UID: plist_from_xml failed (err=%d)\n", err);
+        plist_free(root);
+        return 0;
+    }
+
+    if (plist_get_node_type(root) != PLIST_DICT) {
+        fprintf(stderr, "nested CF$UID: root is not dict\n");
+        plist_free(root);
+        return 0;
+    }
+
+    plist_t obj = plist_dict_get_item(root, "obj");
+    if (!obj) {
+        fprintf(stderr, "nested CF$UID: missing key 'obj'\n");
+        plist_free(root);
+        return 0;
+    }
+
+    if (plist_get_node_type(obj) != PLIST_UID) {
+        fprintf(stderr, "nested CF$UID: expected PLIST_UID, got %d\n",
+                plist_get_node_type(obj));
+        plist_free(root);
+        return 0;
+    }
+
+    uint64_t uid = 0;
+    plist_get_uid_val(obj, &uid);
+    if (uid != 7) {
+        fprintf(stderr, "nested CF$UID: expected uid=7, got %" PRIu64 "\n", uid);
+        plist_free(root);
+        return 0;
+    }
+
+    plist_free(root);
+    return 1;
+}
+
+static int test_extra_root_value_is_rejected(void)
+{
+    /* Two root values inside <plist> must be rejected */
+    const char *xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+        "<plist version=\"1.0\">"
+        "  <string>one</string>"
+        "  <string>two</string>"
+        "</plist>";
+
+    plist_t root = NULL;
+    plist_err_t err = plist_from_xml(xml, (uint32_t)strlen(xml), &root);
+
+    /* Must fail, and root must be NULL (consistent with other parsers) */
+    if (err == PLIST_ERR_SUCCESS || root != NULL) {
+        fprintf(stderr, "extra root value: expected failure, got err=%d root=%p\n",
+                err, (void*)root);
+        plist_free(root);
+        return 0;
+    }
+    return 1;
+}
+
+static int test_scalar_then_extra_node_is_rejected(void)
+{
+    /* Scalar root followed by another node must be rejected */
+    const char *xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+        "<plist version=\"1.0\">"
+        "  <true/>"
+        "  <dict><key>A</key><string>x</string></dict>"
+        "</plist>";
+
+    plist_t root = NULL;
+    plist_err_t err = plist_from_xml(xml, (uint32_t)strlen(xml), &root);
+
+    if (err == PLIST_ERR_SUCCESS || root != NULL) {
+        fprintf(stderr, "scalar then extra node: expected failure, got err=%d root=%p\n",
+                err, (void*)root);
+        plist_free(root);
+        return 0;
+    }
+    return 1;
+}
+
+static int test_scalar_with_comment_is_ok(void)
+{
+    /* Comment after the single root value is not an extra value node */
+    const char *xml =
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+        "<plist version=\"1.0\">"
+        "  <string>ok</string>"
+        "  <!-- trailing comment -->"
+        "</plist>";
+
+    plist_t root = NULL;
+    plist_err_t err = plist_from_xml(xml, (uint32_t)strlen(xml), &root);
+    if (err != PLIST_ERR_SUCCESS || !root) {
+        fprintf(stderr, "scalar + comment: expected success, got err=%d\n", err);
+        plist_free(root);
+        return 0;
+    }
+    if (plist_get_node_type(root) != PLIST_STRING) {
+        fprintf(stderr, "scalar + comment: expected root string, got %d\n",
+                plist_get_node_type(root));
+        plist_free(root);
+        return 0;
+    }
+    plist_free(root);
+    return 1;
+}
+
+int main(void)
+{
+    int ok = 1;
+
+    ok &= test_nested_cfuid_converts_to_uid();
+    ok &= test_extra_root_value_is_rejected();
+    ok &= test_scalar_then_extra_node_is_rejected();
+    ok &= test_scalar_with_comment_is_ok();
+
+    return ok ? 0 : 1;
+}