lib: updatehub: Improve probe security

Improve buffer overflow security on probe_cb. This ensures that socket
buffer have fixed lenght and content received by COAP fills properly on
metadata buffer. After that, ensures that metadata content is a valid
string with length lower than metadata size.

Signed-off-by: Gerson Fernando Budke <[email protected]>

Author: nandojve

lib/updatehub/updatehub.c

@@ -587,22 +587,23 @@ static int report(enum updatehub_state state)
 	return ret;
 }
 
-static void probe_cb(char *metadata)
+static void probe_cb(char *metadata, size_t metadata_size)
 {
 	struct coap_packet reply;
-	char tmp[MAX_PAYLOAD_SIZE];
+	char tmp[MAX_DOWNLOAD_DATA];
+	size_t tmp_len;
 	int rcvd = -1;
 
 	wait_fds();
 
-	rcvd = recv(ctx.sock, metadata, MAX_PAYLOAD_SIZE, MSG_DONTWAIT);
+	rcvd = recv(ctx.sock, tmp, MAX_DOWNLOAD_DATA, MSG_DONTWAIT);
 	if (rcvd <= 0) {
 		LOG_ERR("Could not receive data");
 		ctx.code_status = UPDATEHUB_NETWORKING_ERROR;
 		return;
 	}
 
-	if (coap_packet_parse(&reply, metadata, rcvd, NULL, 0) < 0) {
+	if (coap_packet_parse(&reply, tmp, rcvd, NULL, 0) < 0) {
 		LOG_ERR("Invalid data received");
 		ctx.code_status = UPDATEHUB_DOWNLOAD_ERROR;
 		return;
@@ -614,10 +615,25 @@ static void probe_cb(char *metadata)
 		return;
 	}
 
-	memset(&tmp, 0, MAX_PAYLOAD_SIZE);
-	memcpy(tmp, reply.data + reply.offset, reply.max_len - reply.offset);
-	memset(metadata, 0, MAX_PAYLOAD_SIZE);
-	memcpy(metadata, tmp, strlen(tmp));
+	/* check if we have buffer space to receive payload */
+	if (metadata_size < (reply.max_len - reply.offset)) {
+		LOG_ERR("There is no buffer available");
+		ctx.code_status = UPDATEHUB_METADATA_ERROR;
+		return;
+	}
+
+	memcpy(metadata, reply.data + reply.offset,
+	       reply.max_len - reply.offset);
+
+	/* ensures payload have a valid string with size lower
+	 * than metadata_size
+	 */
+	tmp_len = strlen(metadata);
+	if (tmp_len >= metadata_size) {
+		LOG_ERR("Invalid metadata data received");
+		ctx.code_status = UPDATEHUB_METADATA_ERROR;
+		return;
+	}
 
 	ctx.code_status = UPDATEHUB_OK;
 
@@ -630,8 +646,8 @@ enum updatehub_response updatehub_probe(void)
 	struct resp_probe_some_boards metadata_some_boards;
 	struct resp_probe_any_boards metadata_any_boards;
 
-	char *metadata = k_malloc(MAX_PAYLOAD_SIZE);
-	char *metadata_copy = k_malloc(MAX_PAYLOAD_SIZE);
+	char *metadata = k_malloc(MAX_DOWNLOAD_DATA);
+	char *metadata_copy = k_malloc(MAX_DOWNLOAD_DATA);
 	char *device_id = k_malloc(DEVICE_ID_HEX_MAX_SIZE);
 	char *firmware_version = k_malloc(BOOT_IMG_VER_STRLEN_MAX);
 
@@ -686,8 +702,7 @@ enum updatehub_response updatehub_probe(void)
 		goto cleanup;
 	}
 
-	memset(metadata, 0, MAX_PAYLOAD_SIZE);
-	probe_cb(metadata);
+	probe_cb(metadata, MAX_DOWNLOAD_DATA);
 
 	if (ctx.code_status != UPDATEHUB_OK) {
 		goto cleanup;