[gupnp] Device,Service,RootDevice: Fix use after free



commit b441cb813c751821f1a379d343274b7b90b71da3
Author: Jens Georg <mail jensge org>
Date:   Sun Jan 2 20:06:26 2022 +0100

    Device,Service,RootDevice: Fix use after free
    
    Devices and Services created by a RootDevice were keeping a pointer to
    their XML element, but not to the document, so it could happen that if
    the service lived longer than the RootDevice, the element pointer was
    invalid.
    
    This is already implemented like this for DeviceProxy and ServiceProxy.
    
    No idea why the description-doc property was introduced additionally
    to the "document" property on DeviceInfo.
    
    Fixes #42

 libgupnp/gupnp-device-info.c      | 25 +++++++------
 libgupnp/gupnp-resource-factory.c | 48 ++++++++++++++++++-------
 libgupnp/gupnp-root-device.c      | 76 +++++++++++++++++----------------------
 3 files changed, 80 insertions(+), 69 deletions(-)
---
diff --git a/libgupnp/gupnp-device-info.c b/libgupnp/gupnp-device-info.c
index c7d9374..9d514ce 100644
--- a/libgupnp/gupnp-device-info.c
+++ b/libgupnp/gupnp-device-info.c
@@ -309,19 +309,18 @@ gupnp_device_info_class_init (GUPnPDeviceInfoClass *klass)
          *
          * Stability: Private
          **/
-        g_object_class_install_property
-                (object_class,
-                 PROP_DOCUMENT,
-                 g_param_spec_object ("document",
-                                      "Document",
-                                      "The XML document related to this "
-                                      "device",
-                                      GUPNP_TYPE_XML_DOC,
-                                      G_PARAM_READWRITE |
-                                      G_PARAM_CONSTRUCT_ONLY |
-                                      G_PARAM_STATIC_NAME |
-                                      G_PARAM_STATIC_NICK |
-                                      G_PARAM_STATIC_BLURB));
+        g_object_class_install_property (
+                object_class,
+                PROP_DOCUMENT,
+                g_param_spec_object ("document",
+                                     "Document",
+                                     "The XML document related to this "
+                                     "device",
+                                     GUPNP_TYPE_XML_DOC,
+                                     G_PARAM_READWRITE | G_PARAM_CONSTRUCT |
+                                             G_PARAM_STATIC_NAME |
+                                             G_PARAM_STATIC_NICK |
+                                             G_PARAM_STATIC_BLURB));
 
         /**
          * GUPnPDeviceInfo:element:
diff --git a/libgupnp/gupnp-resource-factory.c b/libgupnp/gupnp-resource-factory.c
index 6d2b798..9e441dc 100644
--- a/libgupnp/gupnp-resource-factory.c
+++ b/libgupnp/gupnp-resource-factory.c
@@ -25,6 +25,7 @@
 #include <config.h>
 #include <string.h>
 
+#include "gupnp-device-info-private.h"
 #include "gupnp-resource-factory-private.h"
 #include "gupnp-root-device.h"
 
@@ -334,14 +335,25 @@ gupnp_resource_factory_create_device (GUPnPResourceFactory *factory,
                                                  element,
                                                  GUPNP_TYPE_DEVICE);
 
+        GUPnPXMLDoc *doc = _gupnp_device_info_get_document (
+                GUPNP_DEVICE_INFO (root_device));
         device = g_object_new (device_type,
-                               "resource-factory", factory,
-                               "context", context,
-                               "root-device", root_device,
-                               "location", location,
-                               "udn", udn,
-                               "url-base", url_base,
-                               "element", element,
+                               "resource-factory",
+                               factory,
+                               "context",
+                               context,
+                               "root-device",
+                               root_device,
+                               "location",
+                               location,
+                               "udn",
+                               udn,
+                               "url-base",
+                               url_base,
+                               "document",
+                               doc,
+                               "element",
+                               element,
                                NULL);
 
         return device;
@@ -390,13 +402,23 @@ gupnp_resource_factory_create_service (GUPnPResourceFactory *factory,
                                                   element,
                                                   GUPNP_TYPE_SERVICE);
 
+        GUPnPXMLDoc *doc = _gupnp_device_info_get_document (
+                GUPNP_DEVICE_INFO (root_device));
         service = g_object_new (service_type,
-                                "context", context,
-                                "root-device", root_device,
-                                "location", location,
-                                "udn", udn,
-                                "url-base", url_base,
-                                "element", element,
+                                "context",
+                                context,
+                                "root-device",
+                                root_device,
+                                "location",
+                                location,
+                                "udn",
+                                udn,
+                                "url-base",
+                                url_base,
+                                "document",
+                                doc,
+                                "element",
+                                element,
                                 NULL);
 
         return service;
diff --git a/libgupnp/gupnp-root-device.c b/libgupnp/gupnp-root-device.c
index 3c337fc..73734a3 100644
--- a/libgupnp/gupnp-root-device.c
+++ b/libgupnp/gupnp-root-device.c
@@ -19,9 +19,10 @@
 
 #include <libgssdp/gssdp-resource-group.h>
 
-#include "gupnp-root-device.h"
 #include "gupnp-context-private.h"
+#include "gupnp-device-info-private.h"
 #include "gupnp-error.h"
+#include "gupnp-root-device.h"
 #include "http-headers.h"
 #include "xml-util.h"
 
@@ -35,8 +36,6 @@ gupnp_root_device_initable_init (GInitable     *initable,
                                  GError       **error);
 
 struct _GUPnPRootDevicePrivate {
-        GUPnPXMLDoc *description_doc;
-
         GSSDPResourceGroup *group;
 
         char  *description_path;
@@ -56,7 +55,6 @@ G_DEFINE_TYPE_EXTENDED (GUPnPRootDevice,
 
 enum {
         PROP_0,
-        PROP_DESCRIPTION_DOC,
         PROP_DESCRIPTION_PATH,
         PROP_DESCRIPTION_DIR,
         PROP_AVAILABLE
@@ -72,7 +70,6 @@ gupnp_root_device_finalize (GObject *object)
         device = GUPNP_ROOT_DEVICE (object);
         priv = gupnp_root_device_get_instance_private (device);
 
-        g_clear_object (&priv->description_doc);
         g_free (priv->description_path);
         g_free (priv->description_dir);
         g_free (priv->relative_location);
@@ -125,9 +122,6 @@ gupnp_root_device_set_property (GObject      *object,
         priv = gupnp_root_device_get_instance_private (device);
 
         switch (property_id) {
-        case PROP_DESCRIPTION_DOC:
-                priv->description_doc = g_value_dup_object (value);
-                break;
         case PROP_DESCRIPTION_PATH:
                 priv->description_path = g_value_dup_string (value);
                 break;
@@ -342,11 +336,13 @@ gupnp_root_device_initable_init (GInitable     *initable,
                                               priv->description_path,
                                               NULL);
 
+        GUPnPXMLDoc *description_doc =
+                _gupnp_device_info_get_document (GUPNP_DEVICE_INFO (device));
         /* Check whether we have a parsed description document */
-        if (priv->description_doc == NULL) {
+        if (description_doc == NULL) {
                 /* We don't, so load and parse it */
-                priv->description_doc = load_and_parse (desc_path);
-                if (priv->description_doc == NULL) {
+                description_doc = load_and_parse (desc_path);
+                if (description_doc == NULL) {
                         g_set_error_literal (error,
                                              GUPNP_XML_ERROR,
                                              GUPNP_XML_ERROR_PARSE,
@@ -354,13 +350,15 @@ gupnp_root_device_initable_init (GInitable     *initable,
 
                         goto DONE;
                 }
+        } else {
+                g_object_ref (description_doc);
         }
 
         /* Find correct element */
-        root_element = xml_util_get_element ((xmlNode *)
-                                             gupnp_xml_doc_get_doc (priv->description_doc),
-                                             "root",
-                                             NULL);
+        root_element = xml_util_get_element (
+                (xmlNode *) gupnp_xml_doc_get_doc (description_doc),
+                "root",
+                NULL);
         if (!root_element) {
                 g_set_error_literal (error,
                                      GUPNP_XML_ERROR,
@@ -418,8 +416,12 @@ gupnp_root_device_initable_init (GInitable     *initable,
 
         /* Set additional properties */
         g_object_set (G_OBJECT (device),
-                      "location", location,
-                      "url-base", url_base,
+                      "location",
+                      location,
+                      "url-base",
+                      url_base,
+                      "document",
+                      description_doc,
                       NULL);
         g_uri_unref (url_base);
 
@@ -443,6 +445,8 @@ gupnp_root_device_initable_init (GInitable     *initable,
         if (uri)
                 g_uri_unref (uri);
 
+        g_clear_object (&description_doc);
+
         g_free (desc_path);
         g_free (location);
 
@@ -461,26 +465,6 @@ gupnp_root_device_class_init (GUPnPRootDeviceClass *klass)
         object_class->dispose      = gupnp_root_device_dispose;
         object_class->finalize     = gupnp_root_device_finalize;
 
-        /**
-         * GUPnPRootDevice:description-doc:
-         *
-         * Device description document. Constructor property.
-         *
-         * Since: 0.14.0
-         **/
-        g_object_class_install_property
-                (object_class,
-                 PROP_DESCRIPTION_DOC,
-                 g_param_spec_object ("description-doc",
-                                      "Description document",
-                                      "Device description document",
-                                      GUPNP_TYPE_XML_DOC,
-                                      G_PARAM_WRITABLE |
-                                      G_PARAM_CONSTRUCT_ONLY |
-                                      G_PARAM_STATIC_NAME |
-                                      G_PARAM_STATIC_NICK |
-                                      G_PARAM_STATIC_BLURB));
-
         /**
          * GUPnPRootDevice:description-path:
          *
@@ -602,12 +586,18 @@ gupnp_root_device_new_full (GUPnPContext         *context,
         return g_initable_new (GUPNP_TYPE_ROOT_DEVICE,
                                NULL,
                                error,
-                               "context", context,
-                               "resource-factory", factory,
-                               "root-device", NULL,
-                               "description-doc", description_doc,
-                               "description-path", description_path,
-                               "description-dir", description_dir,
+                               "context",
+                               context,
+                               "resource-factory",
+                               factory,
+                               "root-device",
+                               NULL,
+                               "document",
+                               description_doc,
+                               "description-path",
+                               description_path,
+                               "description-dir",
+                               description_dir,
                                NULL);
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]