From ee78b90c8cca630e98aeb11060d6ef135abd17df Mon Sep 17 00:00:00 2001
From: Jed Lu <jianlu@amazon.com>
Date: Wed, 7 Oct 2020 17:51:58 +0100
Subject: [PATCH] ss: take care to free any metadata heap values before
 overwrite

Break out the core ss_set_metadata action into a subfunction that
takes the lws_ss_metadata_t, and is fixed to retire heap-based
values before they go out of scope, and adapt the exported version
to call through to that.

Simplify extract_metadata() to reuse the subfunction as well, in
both well-known and custom header cases.
---
 lib/secure-streams/policy-common.c            | 32 +++++++++++++++----
 .../private-lib-secure-streams.h              |  4 +++
 lib/secure-streams/protocols/ss-h1.c          | 19 ++++++-----
 3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/lib/secure-streams/policy-common.c b/lib/secure-streams/policy-common.c
index a2e48ac54..8b0871858 100644
--- a/lib/secure-streams/policy-common.c
+++ b/lib/secure-streams/policy-common.c
@@ -55,6 +55,30 @@ lws_ss_policy_lookup(const struct lws_context *context, const char *streamtype)
 	return NULL;
 }
 
+int
+_lws_ss_set_metadata(lws_ss_metadata_t *omd, const char *name,
+		     const void *value, size_t len)
+{
+	/*
+	 * If there was already a heap-based value, it's about to go out of
+	 * scope due to us trashing the pointer.  So free it first and clear
+	 * its flag indicating it's heap-based.
+	 */
+
+	if (omd->value_on_lws_heap) {
+		lws_free_set_NULL(omd->value);
+		omd->value_on_lws_heap = 0;
+	}
+
+	// lwsl_notice("%s: %s %s\n", __func__, name, (const char *)value);
+
+	omd->name = name;
+	omd->value = (void *)value;
+	omd->length = len;
+
+	return 0;
+}
+
 int
 lws_ss_set_metadata(struct lws_ss_handle *h, const char *name,
 		    const void *value, size_t len)
@@ -66,13 +90,7 @@ lws_ss_set_metadata(struct lws_ss_handle *h, const char *name,
 		return 1;
 	}
 
-	// lwsl_notice("%s: %s %s\n", __func__, name, (const char *)value);
-
-	omd->name = name;
-	omd->value = (void *)value;
-	omd->length = len;
-
-	return 0;
+	return _lws_ss_set_metadata(omd, name, value, len);
 }
 
 int
diff --git a/lib/secure-streams/private-lib-secure-streams.h b/lib/secure-streams/private-lib-secure-streams.h
index f90b3059b..0b69a6347 100644
--- a/lib/secure-streams/private-lib-secure-streams.h
+++ b/lib/secure-streams/private-lib-secure-streams.h
@@ -424,6 +424,10 @@ int
 lws_ss_exp_cb_metadata(void *priv, const char *name, char *out, size_t *pos,
 			size_t olen, size_t *exp_ofs);
 
+int
+_lws_ss_set_metadata(lws_ss_metadata_t *omd, const char *name,
+		     const void *value, size_t len);
+
 int
 _lws_ss_client_connect(lws_ss_handle_t *h, int is_retry);
 
diff --git a/lib/secure-streams/protocols/ss-h1.c b/lib/secure-streams/protocols/ss-h1.c
index 14b9bbf8d..c3426a8d8 100644
--- a/lib/secure-streams/protocols/ss-h1.c
+++ b/lib/secure-streams/protocols/ss-h1.c
@@ -243,23 +243,25 @@ lws_extract_metadata(lws_ss_handle_t *h, struct lws *wsi)
 			if (n) {
 				const char *cp = lws_hdr_simple_ptr(wsi,
 						polmd->value_is_http_token);
+				omd = lws_ss_get_handle_metadata(h, polmd->name);
+				if (!omd)
+					return 1;
+
+				assert(!strcmp(omd->name, polmd->name));
 
 				/*
 				 * it's present on the wsi, we want to
 				 * set the related metadata name to it then
 				 */
 
-				lws_ss_set_metadata(h, polmd->name, cp, n);
+				_lws_ss_set_metadata(omd, polmd->name, cp, n);
 
+#if defined(LWS_WITH_SECURE_STREAMS_PROXY_API)
 				/*
 				 * ...and because we are doing it from parsing
 				 * onward rx, we want to mark the metadata as
 				 * needing passing to the client
 				 */
-
-				omd = lws_ss_get_handle_metadata(h, polmd->name);
-				assert(!strcmp(omd->name, polmd->name));
-#if defined(LWS_WITH_SECURE_STREAMS_PROXY_API)
 				omd->pending_onward = 1;
 #endif
 			}
@@ -309,12 +311,13 @@ lws_extract_metadata(lws_ss_handle_t *h, struct lws *wsi)
 					omd = lws_ss_get_handle_metadata(h,
 								   polmd->name);
 
+					_lws_ss_set_metadata(omd, polmd->name,
+							     p, (size_t)n);
+					omd->value_on_lws_heap = 1;
+
 #if defined(LWS_WITH_SECURE_STREAMS_PROXY_API)
 					omd->pending_onward = 1;
 #endif
-					omd->value = p;
-					omd->length = (size_t)n;
-					omd->value_on_lws_heap = 1;
 				}
 			}
 #endif