From 48dc4538ce2d26b8a3603be57f7d750c80ae2b6b Mon Sep 17 00:00:00 2001
From: Olaf Bergmann <bergmann@tzi.org>
Date: Fri, 19 Jun 2020 16:27:51 +0200
Subject: [PATCH] resource.[hc], net.c: fix use after free for query string

If a resource is observed, handle_request() passes the ownership
of a Uri-Query to the observer structure for later retrieval. In
this case, the query is not free'd in handle_request. However, if
the observer data structure ("subscription") was not created due
to insufficient memory, the query string must not be deleted as
it is used for the resource handler callback later in
handle_request(). The solution here passes ownership to the
observer only if case coap_add_observer() is successful.

This fixes CID 1483551.
---
 include/coap2/resource.h | 3 ++-
 src/net.c                | 5 ++++-
 src/resource.c           | 5 +++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/coap2/resource.h b/include/coap2/resource.h
index c7239b13..b81dbb91 100644
--- a/include/coap2/resource.h
+++ b/include/coap2/resource.h
@@ -419,7 +419,8 @@ coap_resource_t *coap_get_resource_from_uri_path(coap_context_t *context,
  * @param session         The observer's session
  * @param token           The token that identifies this subscription.
  * @param query           The query string, if any. subscription will
-                          take ownership of the string.
+ *                        take ownership of the string unless this
+ *                        function returns NULL.
  * @param has_block2      If Option Block2 defined.
  * @param block2          Contents of Block2 if Block 2 defined.
  * @return                A pointer to the added/updated subscription
diff --git a/src/net.c b/src/net.c
index e4e977a2..035ae836 100644
--- a/src/net.c
+++ b/src/net.c
@@ -2278,8 +2278,11 @@ handle_request(coap_context_t *context, coap_session_t *session, coap_pdu_t *pdu
               has_block2 = 1;
             }
             subscription = coap_add_observer(resource, session, &token, query, has_block2, block2);
-            owns_query = 0;
             if (subscription) {
+              /* Ownership of query is taken by subscription if not
+               * NULL. In this case, we must not delete query here
+               * hence owns_query is cleared. */
+              owns_query = 0;
               coap_touch_observer(context, session, &token);
             }
           }
diff --git a/src/resource.c b/src/resource.c
index 5eda13dc..4a5b7a76 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -674,8 +674,9 @@ coap_add_observer(coap_resource_t *resource,
   s = COAP_MALLOC_TYPE(subscription);
 
   if (!s) {
-    if (query)
-      coap_delete_string(query);
+    /* query is not deleted so it can be used in the calling function
+     * which must give up ownership of query only if this function
+     * does not return NULL. */
     return NULL;
   }