When working with VirtualBox's snapshots, the snapshot XML is
firstly parsed, stored in memory (with some parts being stored as
verbatim XML snippets, strings), requested changes are made and
then this modified XML is formatted via
virVBoxSnapshotConfSaveVboxFile() which calls
xmlParseInNodeContext() to format those previously stored XML
snippets.

The first parse of whole VirtualBox snapshot file is done using
virXMLParse() (in virVBoxSnapshotConfLoadVboxFile()) and thus
with XML_PARSE_NONET specified.

But those ad-hoc parsings when formatting the XML back pass zero
flags mask: xmlParseInNodeContext(..., options = 0, ...);

This is potentially dangerous.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/vbox/vbox_snapshot_conf.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index 84f7aceac2..467255f77f 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -369,6 +369,7 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node,
     int firstRegexResult = 0;
     g_auto(GStrv) secondRegex = NULL;
     int secondRegexResult = 0;
+    const int parseFlags = XML_PARSE_NONET;
 
     uuid = g_strdup_printf("{%s}", snapshot->uuid);
 
@@ -406,7 +407,7 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node,
     parseError = xmlParseInNodeContext(node,
                                        snapshot->hardware,
                                        (int)strlen(snapshot->hardware),
-                                       0,
+                                       parseFlags,
                                        &hardwareNode);
     if (parseError != XML_ERR_OK) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -418,7 +419,7 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node,
     /* storageController */
     if (xmlParseInNodeContext(node, snapshot->storageController,
                               (int)strlen(snapshot->storageController),
-                              0,
+                              parseFlags,
                               &storageControllerNode) != XML_ERR_OK) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("Unable to add the snapshot storageController"));
@@ -944,6 +945,7 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine,
     int firstRegexResult = 0;
     g_auto(GStrv) secondRegex = NULL;
     int secondRegexResult = 0;
+    const int parseFlags = XML_PARSE_NONET;
 
     if (machine == NULL) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1051,7 +1053,7 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine,
         parseError = xmlParseInNodeContext(mediaRegistryNode,
                               machine->mediaRegistry->otherMedia[i],
                               (int)strlen(machine->mediaRegistry->otherMedia[i]),
-                              0,
+                              parseFlags,
                               &cur);
         if (parseError != XML_ERR_OK) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1071,7 +1073,7 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine,
     parseError = xmlParseInNodeContext(machineNode,
                                        machine->hardware,
                                        (int)strlen(machine->hardware),
-                                       0,
+                                       parseFlags,
                                        &cur);
     if (parseError != XML_ERR_OK) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1084,7 +1086,7 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine,
         parseError = xmlParseInNodeContext(xmlDocGetRootElement(xml),
                                            machine->extraData,
                                            (int)strlen(machine->extraData),
-                                           0,
+                                           parseFlags,
                                            &cur);
         if (parseError != XML_ERR_OK) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1097,7 +1099,7 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine,
     parseError = xmlParseInNodeContext(machineNode,
                                        machine->storageController,
                                        (int)strlen(machine->storageController),
-                                       0,
+                                       parseFlags,
                                        &cur);
     if (parseError != XML_ERR_OK) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On a Tuesday in 2023, Michal Privoznik wrote:
>When working with VirtualBox's snapshots, the snapshot XML is
>firstly parsed, stored in memory (with some parts being stored as
>verbatim XML snippets, strings), requested changes are made and
>then this modified XML is formatted via
>virVBoxSnapshotConfSaveVboxFile() which calls
>xmlParseInNodeContext() to format those previously stored XML
>snippets.
>
>The first parse of whole VirtualBox snapshot file is done using
>virXMLParse() (in virVBoxSnapshotConfLoadVboxFile()) and thus
>with XML_PARSE_NONET specified.
>
>But those ad-hoc parsings when formatting the XML back pass zero
>flags mask: xmlParseInNodeContext(..., options = 0, ...);
>
>This is potentially dangerous.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/vbox/vbox_snapshot_conf.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org