From cc1763399c8d23b0cf3af82bc2d4b0a7a28032ae Mon Sep 17 00:00:00 2001 From: suvendhu Date: Thu, 3 Mar 2022 15:41:20 +0530 Subject: [PATCH] Fixed Flawfinder Errors (cherry picked from commit 2649eca7dd5a25cc04da30525a39bc9bd058fe92) --- backupSession.c | 64 ++++++++++++++++++++++----------------------- common.c | 2 +- cwmp.c | 11 +++++--- digestauth.c | 20 ++++++++------ gitlab-ci/shared.sh | 2 +- notifications.c | 19 +++++++++----- rpc_soap.c | 3 +-- ssl_utils.c | 8 +++--- ssl_utils.h | 2 +- xml.c | 15 ++++++++--- 10 files changed, 85 insertions(+), 61 deletions(-) diff --git a/backupSession.c b/backupSession.c index 72cadf2..f58c674 100644 --- a/backupSession.c +++ b/backupSession.c @@ -341,7 +341,7 @@ void bkp_session_insert_schedule_inform(time_t time, char *command_key) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)time); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)time); struct search_keywords sched_inf_insert_keys[2] = { { "command_key", command_key }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "schedule_inform", sched_inf_insert_keys, 2); if (!b) { @@ -358,7 +358,7 @@ void bkp_session_delete_schedule_inform(time_t time, char *command_key) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)time); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)time); struct search_keywords sched_inf_del_keys[2] = { { "command_key", command_key }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "schedule_inform", sched_inf_del_keys, 2); if (b) @@ -373,8 +373,8 @@ void bkp_session_insert_download(struct download *pdownload) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pdownload->scheduled_time); - sprintf(file_size, "%d", pdownload->file_size); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pdownload->scheduled_time); + snprintf(file_size, sizeof(file_size), "%d", pdownload->file_size); struct search_keywords download_insert_keys[7] = { { "url", pdownload->url }, { "command_key", pdownload->command_key }, { "file_type", pdownload->file_type }, { "username", pdownload->username }, { "password", pdownload->password }, { "file_size", file_size }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "download", download_insert_keys, 7); @@ -400,11 +400,11 @@ void bkp_session_insert_schedule_download(struct download *pschedule_download) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(file_size, "%d", pschedule_download->file_size); + snprintf(file_size, sizeof(file_size), "%d", pschedule_download->file_size); for (i = 0; i < 2; i++) { - sprintf(delay[2 * i], "%lld", (long long int)pschedule_download->timewindowstruct[i].windowstart); - sprintf(delay[2 * i + 1], "%lld", (long long int)pschedule_download->timewindowstruct[i].windowend); - sprintf(maxretrie[i], "%d", pschedule_download->timewindowstruct[i].maxretries); + snprintf(delay[2 * i], sizeof(delay[i]), "%lld", (long long int)pschedule_download->timewindowstruct[i].windowstart); + snprintf(delay[2 * i + 1], sizeof(delay[i]), "%lld", (long long int)pschedule_download->timewindowstruct[i].windowend); + snprintf(maxretrie[i], sizeof(maxretrie[i]), "%d", pschedule_download->timewindowstruct[i].maxretries); } struct search_keywords sched_download_insert_keys[16] = { { "url", pschedule_download->url }, { "command_key", pschedule_download->command_key }, @@ -456,9 +456,9 @@ void bkp_session_insert_apply_schedule_download(struct apply_schedule_download * pthread_mutex_lock(&mutex_backup_session); for (i = 0; i < 2; i++) { - sprintf(delay[2 * i], "%lld", (long long int)papply_schedule_download->timeintervals[i].windowstart); - sprintf(delay[2 * i + 1], "%lld", (long long int)papply_schedule_download->timeintervals[i].windowend); - sprintf(maxretrie[i], "%d", papply_schedule_download->timeintervals[i].maxretries); + snprintf(delay[2 * i], sizeof(delay[i]), "%lld", (long long int)papply_schedule_download->timeintervals[i].windowstart); + snprintf(delay[2 * i + 1], sizeof(delay[i]), "%lld", (long long int)papply_schedule_download->timeintervals[i].windowend); + snprintf(maxretrie[i], sizeof(maxretrie[i]), "%d", papply_schedule_download->timeintervals[i].maxretries); } struct search_keywords sched_download_insert_app_keys[9] = { { "command_key", papply_schedule_download->command_key }, @@ -498,9 +498,9 @@ void bkp_session_delete_apply_schedule_download(struct apply_schedule_download * pthread_mutex_lock(&mutex_backup_session); for (i = 0; i < 2; i++) { - sprintf(delay[2 * i], "%lld", (long long int)ps_download->timeintervals[i].windowstart); - sprintf(delay[2 * i + 1], "%lld", (long long int)ps_download->timeintervals[i].windowend); - sprintf(maxretrie[i], "%d", ps_download->timeintervals[i].maxretries); + snprintf(delay[2 * i], sizeof(delay[i]), "%lld", (long long int)ps_download->timeintervals[i].windowstart); + snprintf(delay[2 * i + 1], sizeof(delay[i]), "%lld", (long long int)ps_download->timeintervals[i].windowend); + snprintf(maxretrie[i], sizeof(maxretrie[i]), "%d", ps_download->timeintervals[i].maxretries); } struct search_keywords sched_download_del_app_keys[9] = { { "start_time", ps_download->start_time }, { "command_key", ps_download->command_key }, @@ -526,7 +526,7 @@ void bkp_session_insert_change_du_state(struct change_du_state *pchange_du_state mxml_node_t *b, *n; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pchange_du_state->timeout); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pchange_du_state->timeout); b = bkp_session_insert(bkp_tree, "change_du_state", NULL); bkp_session_insert(b, "command_key", pchange_du_state->command_key); bkp_session_insert(b, "time", schedule_time); @@ -562,7 +562,7 @@ void bkp_session_delete_change_du_state(struct change_du_state *pchange_du_state mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pchange_du_state->timeout); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pchange_du_state->timeout); struct search_keywords cds_del_keys[2] = { { "command_key", pchange_du_state->command_key }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "change_du_state", cds_del_keys, 2); if (b) @@ -576,7 +576,7 @@ void bkp_session_insert_upload(struct upload *pupload) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pupload->scheduled_time); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pupload->scheduled_time); struct search_keywords upload_insert_keys[6] = { { "url", pupload->url }, { "command_key", pupload->command_key }, { "username", pupload->username }, { "password", pupload->password }, { "time", schedule_time }, { "file_type", pupload->file_type } }; b = bkp_session_node_found(bkp_tree, "upload", upload_insert_keys, 6); @@ -598,8 +598,8 @@ void bkp_session_delete_download(struct download *pdownload) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pdownload->scheduled_time); - sprintf(file_size, "%d", pdownload->file_size); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pdownload->scheduled_time); + snprintf(file_size, sizeof(file_size), "%d", pdownload->file_size); struct search_keywords download_del_keys[7] = { { "url", pdownload->url }, { "command_key", pdownload->command_key }, { "file_type", pdownload->file_type }, { "username", pdownload->username }, { "password", pdownload->password }, { "file_size", file_size }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "download", download_del_keys, 7); @@ -617,11 +617,11 @@ void bkp_session_delete_schedule_download(struct download *pschedule_download_de mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(file_size, "%d", pschedule_download_delete->file_size); + snprintf(file_size, sizeof(file_size), "%d", pschedule_download_delete->file_size); for (i = 0; i < 2; i++) { - sprintf(delay[2 * i], "%lld", (long long int)pschedule_download_delete->timewindowstruct[i].windowstart); - sprintf(delay[2 * i + 1], "%lld", (long long int)pschedule_download_delete->timewindowstruct[i].windowend); - sprintf(maxretrie[i], "%d", pschedule_download_delete->timewindowstruct[i].maxretries); + snprintf(delay[2 * i], sizeof(delay[i]), "%lld", (long long int)pschedule_download_delete->timewindowstruct[i].windowstart); + snprintf(delay[2 * i + 1], sizeof(delay[i]), "%lld", (long long int)pschedule_download_delete->timewindowstruct[i].windowend); + snprintf(maxretrie[i], sizeof(maxretrie[i]), "%d", pschedule_download_delete->timewindowstruct[i].maxretries); } struct search_keywords sched_download_del_keys[16] = { { "url", pschedule_download_delete->url }, { "command_key", pschedule_download_delete->command_key }, @@ -652,7 +652,7 @@ void bkp_session_delete_upload(struct upload *pupload) mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pupload->scheduled_time); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pupload->scheduled_time); struct search_keywords upload_del_keys[6] = { { "url", pupload->url }, { "command_key", pupload->command_key }, { "file_type", pupload->file_type }, { "username", pupload->username }, { "password", pupload->password }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "upload", upload_del_keys, 6); if (b) @@ -667,15 +667,15 @@ void bkp_session_insert_du_state_change_complete(struct du_state_change_complete mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pdu_state_change_complete->timeout); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pdu_state_change_complete->timeout); b = bkp_session_insert(bkp_tree, "du_state_change_complete", NULL); bkp_session_insert(b, "command_key", pdu_state_change_complete->command_key); bkp_session_insert(b, "time", schedule_time); list_for_each_entry (p, &(pdu_state_change_complete->list_opresult), list) { mxml_node_t *n; n = bkp_session_insert(b, "opresult", NULL); - sprintf(resolved, "%d", p->resolved); - sprintf(fault_code, "%d", p->fault); + snprintf(resolved, sizeof(resolved), "%d", p->resolved); + snprintf(fault_code, sizeof(fault_code), "%d", p->fault); bkp_session_insert(n, "uuid", p->uuid); bkp_session_insert(n, "du_ref", p->du_ref); bkp_session_insert(n, "version", p->version); @@ -695,7 +695,7 @@ void bkp_session_delete_du_state_change_complete(struct du_state_change_complete char schedule_time[128]; pthread_mutex_lock(&mutex_backup_session); - sprintf(schedule_time, "%lld", (long long int)pdu_state_change_complete->timeout); + snprintf(schedule_time, sizeof(schedule_time), "%lld", (long long int)pdu_state_change_complete->timeout); struct search_keywords cds_complete_keys[2] = { { "command_key", pdu_state_change_complete->command_key }, { "time", schedule_time } }; b = bkp_session_node_found(bkp_tree, "du_state_change_complete", cds_complete_keys, 2); @@ -711,7 +711,7 @@ void bkp_session_insert_transfer_complete(struct transfer_complete *ptransfer_co mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(fault_code, "%d", ptransfer_complete->fault_code); + snprintf(fault_code, sizeof(fault_code), "%d", ptransfer_complete->fault_code); keys[0].name = "command_key"; keys[0].value = ptransfer_complete->command_key; keys[1].name = "start_time"; @@ -721,7 +721,7 @@ void bkp_session_insert_transfer_complete(struct transfer_complete *ptransfer_co keys[3].name = "fault_code"; keys[3].value = fault_code; keys[4].name = "type"; - sprintf(type, "%d", ptransfer_complete->type); + snprintf(type, sizeof(type), "%d", ptransfer_complete->type); keys[4].value = type; b = bkp_session_node_found(bkp_tree, "transfer_complete", keys, 5); if (!b) { @@ -743,8 +743,8 @@ void bkp_session_delete_transfer_complete(struct transfer_complete *ptransfer_co mxml_node_t *b; pthread_mutex_lock(&mutex_backup_session); - sprintf(fault_code, "%d", ptransfer_complete->fault_code); - sprintf(type, "%d", ptransfer_complete->type); + snprintf(fault_code, sizeof(fault_code), "%d", ptransfer_complete->fault_code); + snprintf(type, sizeof(type), "%d", ptransfer_complete->type); struct search_keywords trans_comp_del_keys[5] = { { "command_key", ptransfer_complete->command_key }, { "start_time", ptransfer_complete->start_time }, { "complete_time", ptransfer_complete->complete_time }, { "fault_code", fault_code }, { "type", type } }; b = bkp_session_node_found(bkp_tree, "transfer_complete", trans_comp_del_keys, 5); diff --git a/common.c b/common.c index fbac93f..da3a015 100755 --- a/common.c +++ b/common.c @@ -659,7 +659,7 @@ char *string_to_hex(const unsigned char *str, size_t size) } for (i = 0; i < size; i++) - sprintf(hex + (i * 2), "%02X", str[i]); + snprintf(hex + (i * 2), 3, "%02X", str[i]); return hex; } diff --git a/cwmp.c b/cwmp.c index d4f63eb..8d56992 100644 --- a/cwmp.c +++ b/cwmp.c @@ -34,6 +34,7 @@ #include "sched_inform.h" #include "rpc_soap.h" #include "digestauth.h" +#include "ssl_utils.h" static pthread_t periodic_event_thread; static pthread_t scheduleInform_thread; @@ -50,7 +51,7 @@ bool g_firewall_restart = false; static int cwmp_get_retry_interval(struct cwmp *cwmp) { - int retry_count = 0; + unsigned int retry_count = 0; double min = 0; double max = 0; int m = cwmp->conf.retry_min_wait_interval; @@ -62,8 +63,12 @@ static int cwmp_get_retry_interval(struct cwmp *cwmp) exp = 10; min = pow(((double)k / 1000), (double)(exp - 1)) * m; max = pow(((double)k / 1000), (double)exp) * m; - srand(time(NULL)); - retry_count = rand() % ((int)max + 1 - (int)min) + (int)min; + char *rand = generate_random_string(4); + if (rand) { + unsigned int dividend = (unsigned int)strtoul(rand, NULL, 16); + retry_count = dividend % ((unsigned int)max + 1 - (unsigned int)min) + (unsigned int)min; + free(rand); + } return (retry_count); } diff --git a/digestauth.c b/digestauth.c index cfe9b94..9739d8f 100644 --- a/digestauth.c +++ b/digestauth.c @@ -92,13 +92,18 @@ static void cvthex(const unsigned char *bin, size_t len, char *hex) * @param realm A string of characters that describes the realm of auth. * @param nonce A pointer to a character array for the nonce to put in */ -static void calculate_nonce(uint32_t nonce_time, const char *method, const char *rnd, unsigned int rnd_size, const char *uri, const char *realm, char *nonce) +static void calculate_nonce(uint32_t nonce_time, const char *method, const char *rnd, unsigned int rnd_size, const char *uri, const char *realm, char *nonce, size_t size) { struct MD5Context md5; unsigned char timestamp[4]; unsigned char tmpnonce[MD5_DIGEST_SIZE]; char timestamphex[sizeof(timestamp) * 2 + 1]; + if (nonce == NULL) + return; + + memset(nonce, 0, size); + md5_init(&md5); timestamp[0] = (nonce_time & 0xff000000) >> 0x18; timestamp[1] = (nonce_time & 0x00ff0000) >> 0x10; @@ -117,7 +122,8 @@ static void calculate_nonce(uint32_t nonce_time, const char *method, const char md5_final(tmpnonce, &md5); cvthex(tmpnonce, sizeof(tmpnonce), nonce); cvthex(timestamp, 4, timestamphex); - strcat(nonce, timestamphex); + size_t len = size - strlen(nonce) - 1; + strncat(nonce, timestamphex, len); } /** @@ -168,15 +174,13 @@ static int lookup_sub_value(char *dest, size_t size, const char *data, const cha if ((0 == strncasecmp(ptr, key, keylen)) && (eq == &ptr[keylen])) { if (NULL == q2) { len = strlen(q1); - strcpy(dest, q1); + snprintf(dest, size, "%s", q1); return len; } else { diff = (q2 - q1) + 1; if (size > diff) size = diff; - size--; - memcpy(dest, q1, size); - dest[size] = '\0'; + snprintf(dest, size, "%s", q1); return size; } } @@ -291,7 +295,7 @@ int http_digest_auth_fail_response(FILE *fp, const char *http_method, const char /* Generating the server nonce */ nonce_key_len = nonce_privacy_key ? strlen(nonce_privacy_key) : 0; - calculate_nonce((uint32_t)mhd_monotonic_time(), http_method, nonce_privacy_key, nonce_key_len, url, realm, nonce); + calculate_nonce((uint32_t)mhd_monotonic_time(), http_method, nonce_privacy_key, nonce_key_len, url, realm, nonce, sizeof(nonce)); /* Building the authentication header */ hlen = snprintf(NULL, 0, "Digest realm=\"%s\",qop=\"auth\",nonce=\"%s\",opaque=\"%s\"", realm, nonce, opaque); @@ -396,7 +400,7 @@ int http_digest_auth_check(const char *http_method, const char *url, const char return MHD_INVALID_NONCE; } nonce_key_len = strlen(nonce_privacy_key); - calculate_nonce(nonce_time, http_method, nonce_privacy_key, nonce_key_len, url, realm, noncehashexp); + calculate_nonce(nonce_time, http_method, nonce_privacy_key, nonce_key_len, url, realm, noncehashexp, sizeof(noncehashexp)); /* * Second level vetting for the nonce validity diff --git a/gitlab-ci/shared.sh b/gitlab-ci/shared.sh index d9b25da..cf34d05 100644 --- a/gitlab-ci/shared.sh +++ b/gitlab-ci/shared.sh @@ -38,7 +38,7 @@ function exec_cmd() function configure_genieacs() { - sleep 3 + sleep 10 echo "create a new user" curl -X POST 'http://localhost:3000/init' -H "Content-Type: application/json" --data '{"users": true, "presets": true, "filters": true, "device": true, "index": true, "overview": true}' >/dev/null 2>&1 check_ret $? diff --git a/notifications.c b/notifications.c index c8046a1..3a91846 100644 --- a/notifications.c +++ b/notifications.c @@ -644,13 +644,18 @@ static void udplw_server_param(struct addrinfo **res) char *calculate_lwnotification_cnonce() { - int i; - char *cnonce = malloc(33 * sizeof(char)); - srand((unsigned int)time(NULL)); - for (i = 0; i < 4; i++) { - sprintf(&(cnonce[i * 8]), "%08x", rand()); + char *cnonce = calloc(33, sizeof(char)); + if (cnonce == NULL) + return NULL; + + char *rand = generate_random_string(16); + if (rand == NULL) { + free(cnonce); + return NULL; } - cnonce[i * 8] = '\0'; + + snprintf(cnonce, 33, "%s", rand); + free(rand); return cnonce; } @@ -693,7 +698,7 @@ void cwmp_lwnotification() udplw_server_param(&servaddr); xml_prepare_lwnotification_message(&msg_out); - message_compute_signature(msg_out, signature); + message_compute_signature(msg_out, signature, sizeof(signature)); snprintf(msg, sizeof(msg), "%s \n %s: %s \n %s: %s \n %s: %zu\n %s: %s\n\n%s", "POST /HTTPS/1.1", "HOST", conf->lw_notification_hostname, "Content-Type", "test/xml; charset=utf-8", "Content-Lenght", strlen(msg_out), "Signature", signature, msg_out); send_udp_message(servaddr, msg); diff --git a/rpc_soap.c b/rpc_soap.c index f6972cb..b9c94a8 100755 --- a/rpc_soap.c +++ b/rpc_soap.c @@ -1005,8 +1005,7 @@ int cwmp_handle_rpc_cpe_get_parameter_attributes(struct session *session, struct goto fault; char notification[2]; - sprintf(notification, "%d", param_value->notification); - notification[1] = '\0'; + snprintf(notification, sizeof(notification), "%d", param_value->notification); n = mxmlNewOpaque(n, notification); if (!n) goto fault; diff --git a/ssl_utils.c b/ssl_utils.c index 4314acb..1d38cd7 100644 --- a/ssl_utils.c +++ b/ssl_utils.c @@ -53,7 +53,7 @@ end: return hex; } -void message_compute_signature(char *msg_out, char *signature) +void message_compute_signature(char *msg_out, char *signature, size_t len) { int i; int result_len = 20; @@ -66,9 +66,11 @@ void message_compute_signature(char *msg_out, char *signature) unsigned int *md_len);*/ result = HMAC(EVP_sha1(), conf->acs_passwd, strlen(conf->acs_passwd), (unsigned char *)msg_out, strlen(msg_out), NULL, NULL); for (i = 0; i < result_len; i++) { - sprintf(&(signature[i * 2]), "%02X", result[i]); + if (len - strlen(signature) < 3) // each time 2 hex chars + '\0' at end so needed space is 3 bytes + break; + + snprintf(&(signature[i * 2]), 3, "%02X", result[i]); } - signature[i * 2] = '\0'; FREE(result); } diff --git a/ssl_utils.h b/ssl_utils.h index 221ba1c..e2d2be3 100644 --- a/ssl_utils.h +++ b/ssl_utils.h @@ -22,6 +22,6 @@ #define _SSL_UTILS char *generate_random_string(size_t size); -void message_compute_signature(char *msg_out, char *signature); +void message_compute_signature(char *msg_out, char *signature, size_t len); #endif diff --git a/xml.c b/xml.c index 1ab2d95..924c320 100644 --- a/xml.c +++ b/xml.c @@ -292,9 +292,16 @@ const char *whitespace_cb(mxml_node_t *node, int where) return NULL; break; case MXML_WS_BEFORE_OPEN: - tab_space[0] = '\0'; - while ((node = node->parent)) - strcat(tab_space, CWMP_MXML_TAB_SPACE); + memset(tab_space, 0, sizeof(tab_space)); + int count = 0; + while ((node = node->parent)) { + count = count + 1; + } + + if (count) { + snprintf(tab_space, sizeof(tab_space), "%*s", (int)(count * sizeof(CWMP_MXML_TAB_SPACE)), ""); + } + return tab_space; case MXML_WS_AFTER_OPEN: return ((!node->child || node->child->type == MXML_ELEMENT) ? "\n" : NULL); @@ -396,6 +403,8 @@ int xml_prepare_lwnotification_message(char **msg_out) goto error; c = (char *)calculate_lwnotification_cnonce(); + if (!c) + goto error; b = mxmlNewOpaque(b, c); free(c); if (!b)