[OpenWrt-Devel] [PATCH] rpcd: uci: fix segfault and double free on set method

[Daniel Danzberger]

Invalid reuse of pointers from uci_ptr can cause the rcpd to segfault on already freed memory.
This bug could be trigged by calling 'set' with emtpy values on multiple non existing or already cleard options.

For example:
 ubus call uci set '{"config":"network","section":"wan","values":{"proto":"static","foo":"", "bar":""}}'

Signed-off-by: Daniel Danzberger <daniel at dd-wrt.com>
---
 uci.c | 55 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/uci.c b/uci.c
index 1587a19..e6ddbb5 100644
--- a/uci.c
+++ b/uci.c
@@ -812,55 +812,58 @@ out:
  *     option of if the existing options value differs from the blob value
  */
 static int
-rpc_uci_merge_set(struct blob_attr *opt, struct uci_ptr *ptr)
+rpc_uci_merge_set(struct blob_attr *opt,
+			struct uci_package *package,
+			const char *section)
 {
+	struct uci_ptr ptr = {};
 	struct blob_attr *cur;
 	int rem, rv;
 
-	ptr->o = NULL;
-	ptr->option = blobmsg_name(opt);
-	ptr->value = NULL;
+	ptr.p = package;
+	ptr.section = section;
+	ptr.option = blobmsg_name(opt);
 
-	if (!rpc_uci_verify_name(ptr->option))
+	if (!rpc_uci_verify_name(ptr.option))
 		return UBUS_STATUS_INVALID_ARGUMENT;
 
-	if (rpc_uci_lookup(ptr) || !ptr->s)
+	if (rpc_uci_lookup(&ptr) || !ptr.s)
 		return UBUS_STATUS_NOT_FOUND;
 
 	if (blobmsg_type(opt) == BLOBMSG_TYPE_ARRAY)
 	{
-		if (ptr->o)
-			uci_delete(cursor, ptr);
+		if (ptr.o)
+			uci_delete(cursor, &ptr);
 
 		rv = UBUS_STATUS_INVALID_ARGUMENT;
 
 		blobmsg_for_each_attr(cur, opt, rem)
 		{
-			if (!rpc_uci_format_blob(cur, &ptr->value))
+			if (!rpc_uci_format_blob(cur, &ptr.value))
 				continue;
 
-			uci_add_list(cursor, ptr);
+			uci_add_list(cursor, &ptr);
 			rv = 0;
 		}
 
 		return rv;
 	}
-	else if (ptr->o && ptr->o->type == UCI_TYPE_LIST)
+	else if (ptr.o && ptr.o->type == UCI_TYPE_LIST)
 	{
-		uci_delete(cursor, ptr);
+		uci_delete(cursor, &ptr);
 
-		if (!rpc_uci_format_blob(opt, &ptr->value))
+		if (!rpc_uci_format_blob(opt, &ptr.value))
 			return UBUS_STATUS_INVALID_ARGUMENT;
 
-		uci_set(cursor, ptr);
+		uci_set(cursor, &ptr);
 	}
 	else
 	{
-		if (!rpc_uci_format_blob(opt, &ptr->value))
+		if (!rpc_uci_format_blob(opt, &ptr.value))
 			return UBUS_STATUS_INVALID_ARGUMENT;
 
-		if (!ptr->o || !ptr->o->v.string || strcmp(ptr->o->v.string, ptr->value))
-			uci_set(cursor, ptr);
+		if (!ptr.o || !ptr.o->v.string || strcmp(ptr.o->v.string, ptr.value))
+			uci_set(cursor, &ptr);
 	}
 
 	return 0;
@@ -875,7 +878,7 @@ rpc_uci_set(struct ubus_context *ctx, struct ubus_object *obj,
 	struct blob_attr *cur;
 	struct uci_package *p = NULL;
 	struct uci_element *e;
-	struct uci_ptr ptr = { 0 };
+	const char *package, *section;
 	int rem, rv, err = 0;
 
 	blobmsg_parse(rpc_uci_set_policy, __RPC_S_MAX, tb,
@@ -892,17 +895,17 @@ rpc_uci_set(struct ubus_context *ctx, struct ubus_object *obj,
 	    !rpc_uci_verify_section(blobmsg_data(tb[RPC_S_SECTION])))
 		return UBUS_STATUS_INVALID_ARGUMENT;
 
-	ptr.package = blobmsg_data(tb[RPC_S_CONFIG]);
+	package = blobmsg_data(tb[RPC_S_CONFIG]);
 
-	if (uci_load(cursor, ptr.package, &p))
+	if (uci_load(cursor, package, &p))
 		return rpc_uci_status();
 
 	if (tb[RPC_S_SECTION])
 	{
-		ptr.section = blobmsg_data(tb[RPC_S_SECTION]);
+		section = blobmsg_data(tb[RPC_S_SECTION]);
 		blobmsg_for_each_attr(cur, tb[RPC_S_VALUES], rem)
 		{
-			rv = rpc_uci_merge_set(cur, &ptr);
+			rv = rpc_uci_merge_set(cur, p, section);
 
 			if (rv)
 				err = rv;
@@ -916,12 +919,9 @@ rpc_uci_set(struct ubus_context *ctx, struct ubus_object *obj,
 			                           tb[RPC_S_TYPE], tb[RPC_S_MATCH]))
 				continue;
 
-			ptr.s = NULL;
-			ptr.section = e->name;
-
 			blobmsg_for_each_attr(cur, tb[RPC_S_VALUES], rem)
 			{
-				rv = rpc_uci_merge_set(cur, &ptr);
+				rv = rpc_uci_merge_set(cur, p, e->name);
 
 				if (rv)
 					err = rv;
@@ -929,9 +929,6 @@ rpc_uci_set(struct ubus_context *ctx, struct ubus_object *obj,
 		}
 	}
 
-	if (!err && !ptr.s)
-		err = UBUS_STATUS_NOT_FOUND;
-
 	if (!err)
 		uci_save(cursor, p);
 
-- 
2.23.0


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel