From bef3daf03406f4f609311aec1802bdfdfa0b4687 Mon Sep 17 00:00:00 2001 From: Omar Kallel Date: Mon, 26 Sep 2022 08:16:47 +0100 Subject: [PATCH] Check NULL Pointer Download/Upload/Notifications --- src/download.c | 53 +++++++++++++++++++++++++++++++++------- src/heartbeat.c | 3 --- src/notifications.c | 59 ++++++++++++++++++++++++++++++++++++++------- src/upload.c | 27 +++++++++++++++++---- 4 files changed, 116 insertions(+), 26 deletions(-) diff --git a/src/download.c b/src/download.c index 55a671a..0c33be2 100644 --- a/src/download.c +++ b/src/download.c @@ -37,6 +37,10 @@ int count_download_queue = 0; */ int download_file(const char *file_path, const char *url, const char *username, const char *password) { + if (url == NULL) { + CWMP_LOG(ERROR, "download %s: no url specified", __FUNCTION__); + return -1; + } int res_code = 0; CURL *curl = curl_easy_init(); if (curl) { @@ -55,6 +59,8 @@ int download_file(const char *file_path, const char *url, const char *username, curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS, 10000L); curl_easy_setopt(curl, CURLOPT_TCP_KEEPALIVE, 1L); curl_easy_setopt(curl, CURLOPT_FTP_SKIP_PASV_IP, 1L); + if (file_path == NULL) + file_path = "/tmp/download_file"; FILE *fp = fopen(file_path, "wb"); if (fp) { curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp); @@ -76,6 +82,10 @@ char *download_file_task_function(char *task) memset(&bbuf, 0, sizeof(struct blob_buf)); blob_buf_init(&bbuf, 0); + if (task == NULL) { + CWMP_LOG(ERROR, "download %s: task is null", __FUNCTION__); + return NULL; + } if (blobmsg_add_json_from_string(&bbuf, task) == false) { blob_buf_free(&bbuf); return NULL; @@ -103,6 +113,10 @@ int download_file_in_subprocess(const char *file_path, const char *url, const ch { subprocess_start(download_file_task_function); + if (url == NULL) { + CWMP_LOG(ERROR, "download %s: url is null"); + return 500; + } struct blob_buf bbuf; memset(&bbuf, 0, sizeof(struct blob_buf)); blob_buf_init(&bbuf, 0); @@ -125,6 +139,10 @@ int download_file_in_subprocess(const char *file_path, const char *url, const ch */ void ubus_check_image_callback(struct ubus_request *req, int type __attribute__((unused)), struct blob_attr *msg) { + if (msg == NULL) { + CWMP_LOG(ERROR, "download %s: msg is null"); + return; + } int *code = (int *)req->priv; const struct blobmsg_policy p[2] = { { "code", BLOBMSG_TYPE_INT32 }, { "stdout", BLOBMSG_TYPE_STRING } }; struct blob_attr *tb[2] = { NULL, NULL }; @@ -155,6 +173,10 @@ int cwmp_check_image() */ void ubus_get_available_bank_callback(struct ubus_request *req, int type __attribute__((unused)), struct blob_attr *msg) { + if (msg == NULL) { + CWMP_LOG(ERROR, "download %s: msg is null"); + return; + } int *bank_id = (int *)req->priv; struct blob_attr *banks = NULL; struct blob_attr *cur; @@ -206,6 +228,10 @@ int get_available_bank_id() */ void ubus_get_bank_status_callback(struct ubus_request *req, int type __attribute__((unused)), struct blob_attr *msg) { + if (msg == NULL) { + CWMP_LOG(ERROR, "download %s: msg is null"); + return; + } struct fwbank_dump *bank = (struct fwbank_dump *)req->priv; bool bank_found = false; struct blob_attr *banks = NULL; @@ -367,6 +393,10 @@ int cwmp_launch_download(struct download *pdownload, char *download_file_name, e if (error != FAULT_CPE_NO_FAULT) goto end_download; + if (pdownload->file_type == NULL) { + error = FAULT_CPE_INVALID_ARGUMENTS; + goto end_download; + } if (strcmp(pdownload->file_type, FIRMWARE_UPGRADE_IMAGE_FILE_TYPE) == 0 || strcmp(pdownload->file_type, STORED_FIRMWARE_IMAGE_FILE_TYPE) == 0) { rename(ICWMP_DOWNLOAD_FILE, FIRMWARE_UPGRADE_IMAGE); if (cwmp_check_image() == 0) { @@ -409,7 +439,7 @@ int cwmp_launch_download(struct download *pdownload, char *download_file_name, e end_download: p = calloc(1, sizeof(struct transfer_complete)); - if (p == NULL) { + if (p == NULL || ptransfer_complete == NULL) { error = FAULT_CPE_INTERNAL_ERROR; return error; } @@ -428,10 +458,14 @@ end_download: char *get_file_name_by_download_url(char *url) { - char *slash = strrchr(url, '/'); - if (slash == NULL) - return NULL; - return slash+1; + if (url == NULL) { + CWMP_LOG(ERROR, "download %s: url is null", __FUNCTION__); + return NULL; + } + char *slash = strrchr(url, '/'); + if (slash == NULL) + return NULL; + return slash+1; } int apply_downloaded_file(struct download *pdownload, char *download_file_name, struct transfer_complete *ptransfer_complete) @@ -518,9 +552,9 @@ struct transfer_complete *set_download_error_transfer_complete(struct download * struct transfer_complete *ptransfer_complete; ptransfer_complete = calloc(1, sizeof(struct transfer_complete)); if (ptransfer_complete != NULL) { - ptransfer_complete->command_key = strdup(pdownload->command_key); + ptransfer_complete->command_key = strdup(pdownload && pdownload->command_key ? pdownload->command_key : ""); ptransfer_complete->start_time = strdup(get_time(time(NULL))); - ptransfer_complete->complete_time = strdup(ptransfer_complete->start_time); + ptransfer_complete->complete_time = strdup(ptransfer_complete->start_time ? ptransfer_complete->start_time : ""); ptransfer_complete->fault_code = ltype == TYPE_DOWNLOAD ? FAULT_CPE_DOWNLOAD_FAILURE : FAULT_CPE_DOWNLOAD_FAIL_WITHIN_TIME_WINDOW; ptransfer_complete->type = ltype; bkp_session_insert_transfer_complete(ptransfer_complete); @@ -614,7 +648,7 @@ int cwmp_scheduled_Download_remove_all() int cwmp_rpc_acs_destroy_data_transfer_complete(struct rpc *rpc) { - if (rpc->extra_data != NULL) { + if (rpc && rpc->extra_data != NULL) { struct transfer_complete *p = (struct transfer_complete *)rpc->extra_data; bkp_session_delete_transfer_complete(p); bkp_session_save(); @@ -623,7 +657,8 @@ int cwmp_rpc_acs_destroy_data_transfer_complete(struct rpc *rpc) FREE(p->complete_time); FREE(p->old_software_version); } - FREE(rpc->extra_data); + if (rpc) + FREE(rpc->extra_data); return 0; } diff --git a/src/heartbeat.c b/src/heartbeat.c index 24b69b6..892c625 100644 --- a/src/heartbeat.c +++ b/src/heartbeat.c @@ -55,10 +55,8 @@ void cwmp_heartbeat_session_timer(struct uloop_timeout *timeout __attribute__(( cwmp_main->session->session_status.is_heartbeat = false; return; } - //struct session_timer_event *heartbeat_inform_event = calloc(1, sizeof(struct session_timer_event)); - uloop_timeout_set(&heartbeat_session_timer, cwmp_main->conf.heartbeat_interval * 1000); cwmp_main->session->session_status.next_heartbeat = false; @@ -82,7 +80,6 @@ void intiate_heartbeat_procedures() uloop_timeout_set(&heartbeat_session_timer, cwmp_heartbeat_session_time() * 1000); } } - } } diff --git a/src/notifications.c b/src/notifications.c index 5da6fb7..7f7abc8 100644 --- a/src/notifications.c +++ b/src/notifications.c @@ -40,6 +40,13 @@ struct cwmp_dm_parameter forced_notifications_parameters[] = { */ static bool parameter_is_subobject_of_parameter(char *parent, char *child) { + if (child == NULL) { + CWMP_LOG(WARNING, "notifications %s: child is null", __FUNCTION__); + return false; + } + if (parent == NULL) + parent = "Device."; + if (strcmp(parent, child) == 0) return false; if (strncmp(parent, child, strlen(parent)) == 0) @@ -51,6 +58,11 @@ int check_parameter_forced_notification(const char *parameter) { int i; + if (parameter == NULL) { + CWMP_LOG(WARNING, "notifications %s: parameter is null", __FUNCTION__); + return 0; + } + for (i = 0; i < (int)ARRAY_SIZE(forced_notifications_parameters); i++) { if (strcmp(forced_notifications_parameters[i].name, parameter) == 0) return forced_notifications_parameters[i].notification; @@ -100,7 +112,7 @@ int add_uci_option_notification(char *parameter_name, int notification) bool check_parent_with_different_notification(char *parameter_name, int notification) { - struct uci_list *list_notif; + struct uci_list *list_notif = NULL; struct uci_element *e = NULL; int i; for (i = 0; i < 7; i++) { @@ -129,6 +141,8 @@ bool update_notifications_list(char *parameter_name, int notification) char *ename = NULL; bool update_ret = true; + if (parameter_name == NULL) + parameter_name = "Device."; for (i = 0; i < 7; i++) { int option_type; option_type = cwmp_uci_get_cwmp_varstate_option_value_list("cwmp", "@notifications[0]", notifications[i], &list_notif); @@ -158,6 +172,8 @@ char *cwmp_set_parameter_attributes(char *parameter_name, int notification) { char *error = NULL; + if (parameter_name == NULL) + parameter_name = "Device."; error = check_valid_parameter_path(parameter_name); if (error != NULL) @@ -182,6 +198,8 @@ int get_parameter_family_notifications(char *parameter_name, struct list_head *c int i, notif_ret = 0; char *parent_param = NULL; + if (parameter_name == NULL) + parameter_name = "Device."; for (i = 0; i < 7; i++) { int option_type; @@ -229,6 +247,10 @@ char *cwmp_get_parameter_attributes(char *parameter_name, struct list_head *para { char *error = NULL; + if (parameters_list == NULL) { + CWMP_LOG(ERROR, "notifications %s: childs_list is null", __FUNCTION__); + return NULL; + } error = check_valid_parameter_path(parameter_name); if (error != NULL) @@ -270,6 +292,11 @@ bool parameter_is_other_notif_object_child(char *parent, char *parameter) struct list_head list_iter, *list_ptr; list_iter.next = list_param_obj_notify.next; list_iter.prev = list_param_obj_notify.prev; + + if (parent == NULL) + parent = "Device."; + if (parameter == NULL) + parameter = "Device."; while (list_iter.prev != &list_param_obj_notify) { struct cwmp_dm_parameter *dm_parameter; if (list_iter.prev == NULL) @@ -289,7 +316,7 @@ bool parameter_is_other_notif_object_child(char *parent, char *parameter) void create_list_param_obj_notify() { - struct uci_list *list_notif; + struct uci_list *list_notif = NULL; struct uci_element *e = NULL; int i; @@ -301,13 +328,13 @@ void create_list_param_obj_notify() uci_foreach_element(list_notif, e) { add_dm_parameter_to_list(&list_param_obj_notify, e->name, "", "", i, false); } + if (option_type == UCI_TYPE_STRING) + cwmp_free_uci_list(list_notif); } - if (option_type == UCI_TYPE_STRING) - cwmp_free_uci_list(list_notif); } } -char* updated_list_param_leaf_notify_with_sub_parameter_list(struct list_head *list_param_leaf_notify, struct cwmp_dm_parameter parent_parameter, void (*update_notify_file_line_arg)(FILE *notify_file, char *param_name, char *param_type, char *param_value, int notification), FILE* notify_file_arg) +char* update_list_param_leaf_notify_with_sub_parameter_list(struct list_head *list_param_leaf_notify, struct cwmp_dm_parameter parent_parameter, void (*update_notify_file_line_arg)(FILE *notify_file, char *param_name, char *param_type, char *param_value, int notification), FILE* notify_file_arg) { struct cwmp_dm_parameter *param_iter = NULL; LIST_HEAD(params_list); @@ -332,13 +359,13 @@ void create_list_param_leaf_notify(struct list_head *list_param_leaf_notify, voi int i; for (i = 0; i < (int)ARRAY_SIZE(forced_notifications_parameters); i++) - updated_list_param_leaf_notify_with_sub_parameter_list(list_param_leaf_notify, forced_notifications_parameters[i], update_notify_file_line_arg, notify_file_arg); + update_list_param_leaf_notify_with_sub_parameter_list(list_param_leaf_notify, forced_notifications_parameters[i], update_notify_file_line_arg, notify_file_arg); list_for_each_entry (param_iter, &list_param_obj_notify, list) { if (param_iter->notification == 0) continue; param_iter->forced_notification_param = false; - updated_list_param_leaf_notify_with_sub_parameter_list(list_param_leaf_notify, *param_iter, update_notify_file_line_arg, notify_file_arg); + update_list_param_leaf_notify_with_sub_parameter_list(list_param_leaf_notify, *param_iter, update_notify_file_line_arg, notify_file_arg); } } @@ -362,13 +389,15 @@ void update_notify_file_line(FILE *notify_file, char *param_name, char *param_ty { if (notify_file == NULL) return; + if (param_name == NULL) + return; struct blob_buf bbuf; memset(&bbuf, 0, sizeof(struct blob_buf)); blob_buf_init(&bbuf, 0); blobmsg_add_string(&bbuf, "parameter", param_name); blobmsg_add_u32(&bbuf, "notification", notification); - blobmsg_add_string(&bbuf, "type", param_type); - blobmsg_add_string(&bbuf, "value", param_value); + blobmsg_add_string(&bbuf, "type", param_type ? param_type : "xsd:string"); + blobmsg_add_string(&bbuf, "value", param_value ? param_value : ""); char *notification_line = blobmsg_format_json(bbuf.head, true); if (notification_line != NULL) { fprintf(notify_file, "%s\n", notification_line); @@ -465,6 +494,14 @@ void load_custom_notify_json() void get_parameter_value_from_parameters_list(struct list_head *params_list, char *parameter_name, char **value, char **type) { struct cwmp_dm_parameter *param_value = NULL; + + if (params_list == NULL) { + CWMP_LOG(ERROR, "notifications %s: params_list is null", __FUNCTION__); + return; + } + if (parameter_name == NULL) + parameter_name = "Device."; + list_for_each_entry (param_value, params_list, list) { if (param_value->name == NULL) continue; @@ -563,6 +600,10 @@ void sotfware_version_value_change(struct transfer_complete *p) { char *current_software_version = NULL; + if (p == NULL) { + CWMP_LOG(ERROR, "notifications %s: p is null", __FUNCTION__); + return; + } if (!p->old_software_version || p->old_software_version[0] == 0) return; diff --git a/src/upload.c b/src/upload.c index 0ee4ec5..96fd6b4 100644 --- a/src/upload.c +++ b/src/upload.c @@ -74,6 +74,13 @@ int upload_file(const char *file_path, const char *url, const char *username, co FILE *fd_upload; struct stat file_info; + if (url == NULL) { + CWMP_LOG(ERROR, "upload %s: url is null", __FUNCTION__); + return -1; + } + if (file_path == NULL) { + file_path = "/tmp/upload_file"; + } stat(file_path, &file_info); fd_upload = fopen(file_path, "rb"); if (fd_upload == NULL) { @@ -84,10 +91,11 @@ int upload_file(const char *file_path, const char *url, const char *username, co curl = curl_easy_init(); if (curl) { - char userpass[256]; - - snprintf(userpass, sizeof(userpass), "%s:%s", username, password); - curl_easy_setopt(curl, CURLOPT_USERPWD, userpass); + if (username != NULL && strlen(username) > 0) { + char userpass[256]; + snprintf(userpass, sizeof(userpass), "%s:%s", username, password ? password : ""); + curl_easy_setopt(curl, CURLOPT_USERPWD, userpass); + } if (strncmp(url, "https://", 8) == 0) curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, false); curl_easy_setopt(curl, CURLOPT_TIMEOUT, CURL_TIMEOUT); @@ -115,6 +123,11 @@ char *upload_file_task_function(char *task) { struct blob_buf bbuf; + + if (task == NULL) { + CWMP_LOG(ERROR, "upload %s: task is null", __FUNCTION__); + return NULL; + } memset(&bbuf, 0, sizeof(struct blob_buf)); blob_buf_init(&bbuf, 0); @@ -143,6 +156,10 @@ char *upload_file_task_function(char *task) int upload_file_in_subprocess(const char *file_path, const char *url, const char *username, const char *password) { + if (url == NULL) { + CWMP_LOG(ERROR, "upload %s: url is null"); + return 500; + } subprocess_start(upload_file_task_function); struct blob_buf bbuf; @@ -219,7 +236,7 @@ int cwmp_launch_upload(struct upload *pupload, struct transfer_complete **ptrans end_upload: p = calloc(1, sizeof(struct transfer_complete)); - if (p == NULL) { + if (p == NULL || ptransfer_complete == NULL) { error = FAULT_CPE_INTERNAL_ERROR; return error; }