From fb48f036fece9d64c4cfc19c52091afad5f42fd9 Mon Sep 17 00:00:00 2001
From: jikai <[email protected]>
Date: Sat, 11 May 2024 03:46:02 +0000
Subject: [PATCH 83/85] fix bug for invalid env write

Signed-off-by: jikai <[email protected]>
---
 src/daemon/modules/spec/specs_extend.c | 57 +++++++++-----------------
 src/utils/cutils/utils_verify.c        | 25 +++++++++++
 src/utils/cutils/utils_verify.h        |  2 +
 3 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/src/daemon/modules/spec/specs_extend.c b/src/daemon/modules/spec/specs_extend.c
index 4c154281..f4208405 100644
--- a/src/daemon/modules/spec/specs_extend.c
+++ b/src/daemon/modules/spec/specs_extend.c
@@ -190,41 +190,33 @@ int make_userns_remap(oci_runtime_spec *container, const char *user_remap)
 static int generate_env_map_from_file(FILE *fp, json_map_string_string *env_map)
 {
     int ret = 0;
-    char *key = NULL;
-    char *value = NULL;
-    char *pline = NULL;
+    __isula_auto_free char *pline = NULL;
     size_t length = 0;
-    char *saveptr = NULL;
-    char empty_str[1] = {'\0'};
 
     while (getline(&pline, &length, fp) != -1) {
+        __isula_auto_free char *key = NULL;
+        __isula_auto_free char *value = NULL;
         util_trim_newline(pline);
         pline = util_trim_space(pline);
         if (pline == NULL || pline[0] == '#') {
             continue;
         }
-        key = strtok_r(pline, "=", &saveptr);
-        value = strtok_r(NULL, "=", &saveptr);
-        // value of an env varible is allowed to be empty
-        value = value ? value : empty_str;
-        if (key != NULL) {
-            key = util_trim_space(key);
-            value = util_trim_space(value);
-            if ((size_t)(MAX_BUFFER_SIZE - 1) - strlen(key) < strlen(value)) {
-                ERROR("env length exceed %d bytes", MAX_BUFFER_SIZE);
-                ret = -1;
-                goto out;
-            }
-            ret = append_json_map_string_string(env_map, key, value);
-            if (ret < 0) {
-                ERROR("append env to map failed");
-                goto out;
-            }
+        if (util_valid_split_env(pline, &key, &value) < 0) {
+            // ignore invalid env
+            continue;
+        }
+        if ((size_t)(MAX_BUFFER_SIZE - 1) - strlen(key) < strlen(value)) {
+            ERROR("env length exceed %d bytes", MAX_BUFFER_SIZE);
+            return -1;
+        }
+        ret = append_json_map_string_string(env_map, key, value);
+        if (ret < 0) {
+            ERROR("append env to map failed");
+            return -1;
         }
     }
-out:
-    free(pline);
-    return ret;
+
+    return 0;
 }
 
 static json_map_string_string *parse_env_target_file(const char *env_path)
@@ -293,28 +285,17 @@ static int do_append_env(char ***env, size_t *env_len, const char *key, const ch
 static int check_env_need_append(const oci_runtime_spec *oci_spec, const char *env_key, bool *is_append)
 {
     size_t i = 0;
-    char *key = NULL;
-    char *saveptr = NULL;
 
     for (i = 0; i < oci_spec->process->env_len; i++) {
-        char *tmp_env = NULL;
-        tmp_env = util_strdup_s(oci_spec->process->env[i]);
-        key = strtok_r(tmp_env, "=", &saveptr);
-        // value of an env varible is allowed to be empty
-        if (key == NULL) {
+        __isula_auto_free char *key = NULL;
+        if (util_valid_split_env(oci_spec->process->env[i], &key, NULL) < 0) {
             ERROR("Bad env format");
-            free(tmp_env);
-            tmp_env = NULL;
             return -1;
         }
         if (strcmp(key, env_key) == 0) {
             *is_append = false;
-            free(tmp_env);
-            tmp_env = NULL;
             return 0;
         }
-        free(tmp_env);
-        tmp_env = NULL;
     }
     return 0;
 }
diff --git a/src/utils/cutils/utils_verify.c b/src/utils/cutils/utils_verify.c
index 474e28f0..6f1da12c 100644
--- a/src/utils/cutils/utils_verify.c
+++ b/src/utils/cutils/utils_verify.c
@@ -651,6 +651,31 @@ bool util_valid_device_cgroup_rule(const char *value)
     return util_reg_match(patten, value) == 0;
 }
 
+int util_valid_split_env(const char *env, char **key, char **value)
+{
+    __isula_auto_array_t char **arr = NULL;
+
+    arr = util_string_split_n(env, '=', 2);
+    if (arr == NULL) {
+        ERROR("Failed to split env string");
+        return -1;
+    }
+
+    if (strlen(arr[0]) == 0) {
+        ERROR("Invalid environment variable: %s", env);
+        return -1;
+    }
+
+    if (key != NULL) {
+        *key = util_strdup_s(arr[0]);
+    }
+    if (value != NULL) {
+        *value = util_strdup_s(util_array_len((const char **)arr) > 1 ? arr[1] : "");
+    }
+
+    return 0;
+}
+
 int util_valid_env(const char *env, char **dst)
 {
     int ret = 0;
diff --git a/src/utils/cutils/utils_verify.h b/src/utils/cutils/utils_verify.h
index fc59f6c0..58b22b85 100644
--- a/src/utils/cutils/utils_verify.h
+++ b/src/utils/cutils/utils_verify.h
@@ -119,6 +119,8 @@ bool util_valid_positive_interger(const char *value);
 
 bool util_valid_device_cgroup_rule(const char *value);
 
+int util_valid_split_env(const char *env, char **key, char **value);
+
 int util_valid_env(const char *env, char **dst);
 
 bool util_valid_sysctl(const char *sysctl_key);
-- 
2.34.1