commit fc852cd9fdcdc2fbac6da7eba4cadbc9615fd653
author Anirudh Mahto <quic_amahto@quicinc.com> Sat Sep 03 20:28:11 2022 +0530
committer Sivanagaraju S <quic_sivanaga@quicinc.com> Wed Dec 14 11:07:53 2022 +0530
tree ad4ba8b088192594880453403fccdce79ab8ab6b
parent eb804d5909382b152c5464a12e133f849e36b8a3

agm: service: Handle metadata parsing within size limit

Add size limit check before parsing GKV ,CKV and properties metadata
to avoid memory read overflow

Change-Id: Ia2b99c7641166ef042b22e5a5581d264e0aa8eea

service/src/metadata.c

diff --git a/service/src/metadata.c b/service/src/metadata.c
index c94f856..047f5e0 100644
--- a/service/src/metadata.c
+++ b/service/src/metadata.c
@@ -270,17 +270,17 @@
     return merged;
 }
 
-int metadata_copy(struct agm_meta_data_gsl *dest, uint32_t size __unused,
+int metadata_copy(struct agm_meta_data_gsl *dest, uint32_t size,
                                               uint8_t *metadata)
 {
 
     int ret = 0;
+    int min_req_len = 0;
 
     if (!metadata) {
         AGM_LOGI("NULL metadata passed, ignoring\n");
-        return ret;
+        goto done;
     }
-
     if ((NUM_GKV(metadata) > MAX_KVPAIR) || (NUM_CKV(metadata) > MAX_KVPAIR)) {
         AGM_LOGE("Num GKVs %d Num CKVs %d more than expected: %d", NUM_GKV(metadata),
                                                       NUM_CKV(metadata), MAX_KVPAIR);
@@ -288,39 +288,84 @@
         return ret;
     }
 
+    min_req_len += sizeof(uint32_t);
+    if (size < min_req_len) {
+        AGM_LOGE("size should be atleast %d size for GKV\n", sizeof(uint32_t));
+        ret = -EINVAL;
+        goto done;
+
+    }
+
     dest->gkv.num_kvs = NUM_GKV(metadata);
     dest->gkv.kv =  calloc(dest->gkv.num_kvs, sizeof(struct agm_key_value));
     if (!dest->gkv.kv) {
         AGM_LOGE("Memory allocation failed to copy GKV\n");
         ret = -ENOMEM;
-        return ret;
+        goto free_metadata;
+    }
+
+    min_req_len += (dest->gkv.num_kvs * sizeof(struct agm_key_value));
+    if (size < min_req_len) {
+        AGM_LOGE("Invalid GKV passed\n");
+        ret = -EINVAL;
+        goto free_metadata;
     }
     memcpy(dest->gkv.kv, PTR_TO_GKV(metadata), dest->gkv.num_kvs *
                                     sizeof(struct agm_key_value));
 
+    min_req_len += sizeof(uint32_t);
+    if (size < min_req_len) {
+        goto done;
+    }
     dest->ckv.num_kvs = NUM_CKV(metadata);
     dest->ckv.kv =  calloc(dest->ckv.num_kvs, sizeof(struct agm_key_value));
     if (!dest->ckv.kv) {
         AGM_LOGE("Memory allocation failed to copy CKV\n");
-        metadata_free(dest);
         ret = -ENOMEM;
-        return ret;
+        goto free_metadata;
+    }
+    min_req_len += (dest->ckv.num_kvs * sizeof(struct agm_key_value));
+    if (size < min_req_len) {
+        AGM_LOGE("Invalid CKV passed\n");
+        ret = -EINVAL;
+        goto free_metadata;
     }
     memcpy(dest->ckv.kv, PTR_TO_CKV(metadata), dest->ckv.num_kvs *
                                     sizeof(struct agm_key_value));
 
+    min_req_len += sizeof(uint32_t);
+    if (size < min_req_len) {
+        goto done;
+    }
     dest->sg_props.prop_id = PROP_ID(metadata);
+
+    min_req_len += sizeof(uint32_t);
+    if (size < min_req_len) {
+        AGM_LOGE("Invalid properties passed\n");
+        ret = -EINVAL;
+        goto free_metadata;
+    }
     dest->sg_props.num_values = NUM_PROPS(metadata);
     dest->sg_props.values =  calloc(dest->sg_props.num_values, sizeof(uint32_t));
     if (!dest->sg_props.values) {
         AGM_LOGE("Memory allocation failed to copy properties\n");
-        metadata_free(dest);
         ret = -ENOMEM;
-        return ret;
+        goto free_metadata;
+    }
+    min_req_len += (dest->sg_props.num_values * sizeof(uint32_t));
+    if (size < min_req_len) {
+        AGM_LOGE("Invalid properties passed\n");
+        ret = -EINVAL;
+        goto free_metadata;
     }
     memcpy(dest->sg_props.values, PTR_TO_PROPS(metadata),
            dest->sg_props.num_values * sizeof(uint32_t));
+    goto done;
 
+free_metadata:
+    metadata_free(dest);
+
+done:
     return ret;
 
 }