From 64e30c0fe3813e626c3506eb8a5a3fea0db4e970 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Tue, 10 Sep 2024 15:02:58 +0200 Subject: [PATCH] Fixed small bugs identified via static analysis (Coverity) --- events/janus_gelfevh.c | 4 ++-- ice.c | 26 +++++++++++++++----------- plugins/janus_audiobridge.c | 3 +++ plugins/janus_duktape.c | 18 ++++++++++++------ plugins/janus_lua.c | 18 ++++++++++++------ plugins/janus_sip.c | 2 +- plugins/janus_videocall.c | 15 ++++++++------- postprocessing/janus-pp-rec.c | 4 ++-- postprocessing/pp-h264.c | 8 +++++--- postprocessing/pp-h265.c | 8 +++++--- postprocessing/pp-webm.c | 10 ++++++++-- transports/janus_http.c | 10 +++++++--- transports/janus_websockets.c | 7 +++---- turnrest.c | 2 +- 14 files changed, 84 insertions(+), 51 deletions(-) diff --git a/events/janus_gelfevh.c b/events/janus_gelfevh.c index 3883795732..887a2b930e 100644 --- a/events/janus_gelfevh.c +++ b/events/janus_gelfevh.c @@ -201,10 +201,10 @@ static int janus_gelfevh_send(char *message) { if(transport == JANUS_GELFEVH_SOCKET_TYPE_TCP) { /* TCP */ int out_bytes = 0; - int length = strlen(message); + int length = strlen(message) + 1; char *buffer = message; while(length > 0) { - out_bytes = send(sockfd, buffer, length + 1, 0); + out_bytes = send(sockfd, buffer, length, 0); if(out_bytes <= 0) { JANUS_LOG(LOG_WARN, "Sending TCP message failed, dropping event: %d (%s)\n", errno, g_strerror(errno)); close(sockfd); diff --git a/ice.c b/ice.c index 4440f86223..477ffcbd8a 100644 --- a/ice.c +++ b/ice.c @@ -363,20 +363,22 @@ void janus_ice_enforce_interface(const char *ip) { janus_mutex_unlock(&ice_list_mutex); } gboolean janus_ice_is_enforced(const char *ip) { - if(ip == NULL || janus_ice_enforce_list == NULL) - return false; janus_mutex_lock(&ice_list_mutex); + if(ip == NULL || janus_ice_enforce_list == NULL) { + janus_mutex_unlock(&ice_list_mutex); + return FALSE; + } GList *temp = janus_ice_enforce_list; while(temp) { const char *enforced = (const char *)temp->data; if(enforced != NULL && strstr(ip, enforced) == ip) { janus_mutex_unlock(&ice_list_mutex); - return true; + return TRUE; } temp = temp->next; } janus_mutex_unlock(&ice_list_mutex); - return false; + return FALSE; } void janus_ice_ignore_interface(const char *ip) { @@ -391,20 +393,22 @@ void janus_ice_ignore_interface(const char *ip) { janus_mutex_unlock(&ice_list_mutex); } gboolean janus_ice_is_ignored(const char *ip) { - if(ip == NULL || janus_ice_ignore_list == NULL) - return false; janus_mutex_lock(&ice_list_mutex); + if(ip == NULL || janus_ice_ignore_list == NULL) { + janus_mutex_unlock(&ice_list_mutex); + return FALSE; + } GList *temp = janus_ice_ignore_list; while(temp) { const char *ignored = (const char *)temp->data; if(ignored != NULL && strstr(ip, ignored) == ip) { janus_mutex_unlock(&ice_list_mutex); - return true; + return TRUE; } temp = temp->next; } janus_mutex_unlock(&ice_list_mutex); - return false; + return FALSE; } @@ -418,7 +422,7 @@ int janus_ice_get_event_stats_period(void) { } /* How to handle media statistic events (one per media or one per peerConnection) */ -static gboolean janus_ice_event_combine_media_stats = false; +static gboolean janus_ice_event_combine_media_stats = FALSE; void janus_ice_event_set_combine_media_stats(gboolean combine_media_stats_to_one_event) { janus_ice_event_combine_media_stats = combine_media_stats_to_one_event; } @@ -3155,7 +3159,7 @@ static void janus_ice_cb_nice_recv(NiceAgent *agent, guint stream_id, guint comp } if(rtcp_ssrc == 0) { if(!fallback) { - fallback = true; + fallback = TRUE; continue; } /* No SSRC, maybe an empty RR? */ @@ -3233,7 +3237,7 @@ static void janus_ice_cb_nice_recv(NiceAgent *agent, guint stream_id, guint comp } if(rtcp_ssrc == 0) { if(!fallback) { - fallback = true; + fallback = TRUE; continue; } /* No SSRC, maybe an empty RR? */ diff --git a/plugins/janus_audiobridge.c b/plugins/janus_audiobridge.c index 3d1ed5ba2e..767f7f72f0 100644 --- a/plugins/janus_audiobridge.c +++ b/plugins/janus_audiobridge.c @@ -7298,6 +7298,7 @@ static void *janus_audiobridge_handler(void *data) { if(error_code != 0) { janus_mutex_unlock(&audiobridge->mutex); janus_refcount_decrease(&audiobridge->ref); + janus_mutex_unlock(&rooms_mutex); goto error; } admin = TRUE; @@ -7311,6 +7312,7 @@ static void *janus_audiobridge_handler(void *data) { if(error_code != 0) { janus_mutex_unlock(&audiobridge->mutex); janus_refcount_decrease(&audiobridge->ref); + janus_mutex_unlock(&rooms_mutex); goto error; } const char *group_name = json_string_value(json_object_get(root, "group")); @@ -7318,6 +7320,7 @@ static void *janus_audiobridge_handler(void *data) { if(group == 0) { janus_mutex_unlock(&audiobridge->mutex); janus_refcount_decrease(&audiobridge->ref); + janus_mutex_unlock(&rooms_mutex); JANUS_LOG(LOG_ERR, "No such group (%s)\n", group_name); error_code = JANUS_AUDIOBRIDGE_ERROR_NO_SUCH_GROUP; g_snprintf(error_cause, 512, "No such group (%s)", group_name); diff --git a/plugins/janus_duktape.c b/plugins/janus_duktape.c index 02d691782d..d09c581841 100644 --- a/plugins/janus_duktape.c +++ b/plugins/janus_duktape.c @@ -1848,11 +1848,12 @@ int janus_duktape_get_version(void) { /* Check if the JS script wants to override this method and return info itself */ if(has_get_version) { /* Yep, pass the request to the JS script and return the info */ + janus_mutex_lock(&duktape_mutex); if(duktape_script_version != -1) { /* Unless we asked already */ + janus_mutex_unlock(&duktape_mutex); return duktape_script_version; } - janus_mutex_lock(&duktape_mutex); duk_idx_t thr_idx = duk_push_thread(duktape_ctx); duk_context *t = duk_get_context(duktape_ctx, thr_idx); duk_get_global_string(t, "getVersion"); @@ -1879,11 +1880,12 @@ const char *janus_duktape_get_version_string(void) { /* Check if the JS script wants to override this method and return info itself */ if(has_get_version_string) { /* Yep, pass the request to the JS script and return the info */ + janus_mutex_lock(&duktape_mutex); if(duktape_script_version_string != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&duktape_mutex); return duktape_script_version_string; } - janus_mutex_lock(&duktape_mutex); duk_idx_t thr_idx = duk_push_thread(duktape_ctx); duk_context *t = duk_get_context(duktape_ctx, thr_idx); duk_get_global_string(t, "getVersionString"); @@ -1912,11 +1914,12 @@ const char *janus_duktape_get_description(void) { /* Check if the JS script wants to override this method and return info itself */ if(has_get_description) { /* Yep, pass the request to the JS script and return the info */ + janus_mutex_lock(&duktape_mutex); if(duktape_script_description != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&duktape_mutex); return duktape_script_description; } - janus_mutex_lock(&duktape_mutex); duk_idx_t thr_idx = duk_push_thread(duktape_ctx); duk_context *t = duk_get_context(duktape_ctx, thr_idx); duk_get_global_string(t, "getDescription"); @@ -1945,11 +1948,12 @@ const char *janus_duktape_get_name(void) { /* Check if the JS script wants to override this method and return info itself */ if(has_get_name) { /* Yep, pass the request to the JS script and return the info */ + janus_mutex_lock(&duktape_mutex); if(duktape_script_name != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&duktape_mutex); return duktape_script_name; } - janus_mutex_lock(&duktape_mutex); duk_idx_t thr_idx = duk_push_thread(duktape_ctx); duk_context *t = duk_get_context(duktape_ctx, thr_idx); duk_get_global_string(t, "getName"); @@ -1978,11 +1982,12 @@ const char *janus_duktape_get_author(void) { /* Check if the JS script wants to override this method and return info itself */ if(has_get_author) { /* Yep, pass the request to the JS script and return the info */ + janus_mutex_lock(&duktape_mutex); if(duktape_script_author != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&duktape_mutex); return duktape_script_author; } - janus_mutex_lock(&duktape_mutex); duk_idx_t thr_idx = duk_push_thread(duktape_ctx); duk_context *t = duk_get_context(duktape_ctx, thr_idx); duk_get_global_string(t, "getAuthor"); @@ -2011,11 +2016,12 @@ const char *janus_duktape_get_package(void) { /* Check if the JS script wants to override this method and return info itself */ if(has_get_package) { /* Yep, pass the request to the JS script and return the info */ + janus_mutex_lock(&duktape_mutex); if(duktape_script_package != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&duktape_mutex); return duktape_script_package; } - janus_mutex_lock(&duktape_mutex); duk_idx_t thr_idx = duk_push_thread(duktape_ctx); duk_context *t = duk_get_context(duktape_ctx, thr_idx); duk_get_global_string(t, "getPackage"); diff --git a/plugins/janus_lua.c b/plugins/janus_lua.c index 7ce5db5da7..9af230eb4e 100644 --- a/plugins/janus_lua.c +++ b/plugins/janus_lua.c @@ -1617,11 +1617,12 @@ int janus_lua_get_version(void) { /* Check if the Lua script wants to override this method and return info itself */ if(has_get_version) { /* Yep, pass the request to the Lua script and return the info */ + janus_mutex_lock(&lua_mutex); if(lua_script_version != -1) { /* Unless we asked already */ + janus_mutex_unlock(&lua_mutex); return lua_script_version; } - janus_mutex_lock(&lua_mutex); lua_State *t = lua_newthread(lua_state); lua_getglobal(t, "getVersion"); lua_call(t, 0, 1); @@ -1638,11 +1639,12 @@ const char *janus_lua_get_version_string(void) { /* Check if the Lua script wants to override this method and return info itself */ if(has_get_version_string) { /* Yep, pass the request to the Lua script and return the info */ + janus_mutex_lock(&lua_mutex); if(lua_script_version_string != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&lua_mutex); return lua_script_version_string; } - janus_mutex_lock(&lua_mutex); lua_State *t = lua_newthread(lua_state); lua_getglobal(t, "getVersionString"); lua_call(t, 0, 1); @@ -1661,11 +1663,12 @@ const char *janus_lua_get_description(void) { /* Check if the Lua script wants to override this method and return info itself */ if(has_get_description) { /* Yep, pass the request to the Lua script and return the info */ + janus_mutex_lock(&lua_mutex); if(lua_script_description != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&lua_mutex); return lua_script_description; } - janus_mutex_lock(&lua_mutex); lua_State *t = lua_newthread(lua_state); lua_getglobal(t, "getDescription"); lua_call(t, 0, 1); @@ -1684,11 +1687,12 @@ const char *janus_lua_get_name(void) { /* Check if the Lua script wants to override this method and return info itself */ if(has_get_name) { /* Yep, pass the request to the Lua script and return the info */ + janus_mutex_lock(&lua_mutex); if(lua_script_name != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&lua_mutex); return lua_script_name; } - janus_mutex_lock(&lua_mutex); lua_State *t = lua_newthread(lua_state); lua_getglobal(t, "getName"); lua_call(t, 0, 1); @@ -1707,11 +1711,12 @@ const char *janus_lua_get_author(void) { /* Check if the Lua script wants to override this method and return info itself */ if(has_get_author) { /* Yep, pass the request to the Lua script and return the info */ + janus_mutex_lock(&lua_mutex); if(lua_script_author != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&lua_mutex); return lua_script_author; } - janus_mutex_lock(&lua_mutex); lua_State *t = lua_newthread(lua_state); lua_getglobal(t, "getAuthor"); lua_call(t, 0, 1); @@ -1730,11 +1735,12 @@ const char *janus_lua_get_package(void) { /* Check if the Lua script wants to override this method and return info itself */ if(has_get_package) { /* Yep, pass the request to the Lua script and return the info */ + janus_mutex_lock(&lua_mutex); if(lua_script_package != NULL) { /* Unless we asked already */ + janus_mutex_unlock(&lua_mutex); return lua_script_package; } - janus_mutex_lock(&lua_mutex); lua_State *t = lua_newthread(lua_state); lua_getglobal(t, "getPackage"); lua_call(t, 0, 1); diff --git a/plugins/janus_sip.c b/plugins/janus_sip.c index 3379b72a98..0966a73049 100644 --- a/plugins/janus_sip.c +++ b/plugins/janus_sip.c @@ -5135,7 +5135,7 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase, break; case nua_i_state:; tagi_t const *ti = tl_find(tags, nutag_callstate); - enum nua_callstate callstate = ti ? ti->t_value : -1; + enum nua_callstate callstate = ti ? ti->t_value : nua_callstate_init; JANUS_LOG(LOG_VERB, "[%s][%s]: %d %s, call state [%s]\n", session->account.username, nua_event_name(event), status, phrase ? phrase : "??", nua_callstate_name(callstate)); /* There are several call states, but we care about the terminated state in order to send the 'hangup' event * and the proceeding state in order to send the 'proceeding' event so the client can play a ringback tone for diff --git a/plugins/janus_videocall.c b/plugins/janus_videocall.c index b1ad1ba490..52f0d9b412 100644 --- a/plugins/janus_videocall.c +++ b/plugins/janus_videocall.c @@ -1136,12 +1136,6 @@ static void *janus_videocall_handler(void *data) { janus_mutex_unlock(&sessions_mutex); } else if(!strcasecmp(request_text, "register")) { /* Map this handle to a username */ - if(session->username != NULL) { - JANUS_LOG(LOG_ERR, "Already registered (%s)\n", session->username); - error_code = JANUS_VIDEOCALL_ERROR_ALREADY_REGISTERED; - g_snprintf(error_cause, 512, "Already registered (%s)", session->username); - goto error; - } JANUS_VALIDATE_JSON_OBJECT(root, username_parameters, error_code, error_cause, TRUE, JANUS_VIDEOCALL_ERROR_MISSING_ELEMENT, JANUS_VIDEOCALL_ERROR_INVALID_ELEMENT); @@ -1150,6 +1144,13 @@ static void *janus_videocall_handler(void *data) { json_t *username = json_object_get(root, "username"); const char *username_text = json_string_value(username); janus_mutex_lock(&sessions_mutex); + if(session->username != NULL) { + janus_mutex_unlock(&sessions_mutex); + JANUS_LOG(LOG_ERR, "Already registered (%s)\n", session->username); + error_code = JANUS_VIDEOCALL_ERROR_ALREADY_REGISTERED; + g_snprintf(error_cause, 512, "Already registered (%s)", session->username); + goto error; + } if(g_hash_table_lookup(usernames, username_text) != NULL) { janus_mutex_unlock(&sessions_mutex); JANUS_LOG(LOG_ERR, "Username '%s' already taken\n", username_text); @@ -1354,7 +1355,7 @@ static void *janus_videocall_handler(void *data) { g_snprintf(error_cause, 512, "Error parsing answer: %s", error_str); goto error; } - JANUS_LOG(LOG_VERB, "%s is accepting a call from %s\n", session->username, peer->username); + JANUS_LOG(LOG_VERB, "%s is accepting a call from %s\n", session->username, peer ? peer->username : "??"); JANUS_LOG(LOG_VERB, "This is involving a negotiation (%s) as well:\n%s\n", msg_sdp_type, msg_sdp); session->has_audio = (strstr(msg_sdp, "m=audio") != NULL); session->has_video = (strstr(msg_sdp, "m=video") != NULL); diff --git a/postprocessing/janus-pp-rec.c b/postprocessing/janus-pp-rec.c index fd7f773190..e03db4ade5 100644 --- a/postprocessing/janus-pp-rec.c +++ b/postprocessing/janus-pp-rec.c @@ -1660,8 +1660,8 @@ static gint janus_pp_skew_compensate_audio(janus_pp_frame_packet *pkt, janus_pp_ exit_status = -1; } else { context->target_ts = 0; - /* Do not execute analysis for out of order packets or multi-packets frame */ - if (context->last_seq == context->prev_seq + 1 && context->last_ts != context->prev_ts) { + /* Do not execute analysis for out of order packets or multi-packets frame or if pts < start_time */ + if (context->last_seq == context->prev_seq + 1 && context->last_ts != context->prev_ts && pts >= context->start_time) { /* Evaluate the local RTP timestamp according to the local clock */ guint64 expected_ts = ((pts - context->start_time) * akhz) + context->start_ts; /* Evaluate current delay */ diff --git a/postprocessing/pp-h264.c b/postprocessing/pp-h264.c index f7f5728b20..63d68f2c02 100644 --- a/postprocessing/pp-h264.c +++ b/postprocessing/pp-h264.c @@ -146,9 +146,11 @@ static uint32_t janus_pp_h264_eg_decode(uint8_t *base, uint32_t *offset) { while(janus_pp_h264_eg_getbit(base, (*offset)++) == 0) zeros++; uint32_t res = 1 << zeros; - int32_t i = 0; - for(i=zeros-1; i>=0; i--) { - res |= janus_pp_h264_eg_getbit(base, (*offset)++) << i; + if(zeros > 0) { + int32_t i = 0; + for(i=zeros-1; i>=0; i--) { + res |= janus_pp_h264_eg_getbit(base, (*offset)++) << i; + } } return res-1; } diff --git a/postprocessing/pp-h265.c b/postprocessing/pp-h265.c index cd09983ab2..eacdd5c5f1 100644 --- a/postprocessing/pp-h265.c +++ b/postprocessing/pp-h265.c @@ -154,9 +154,11 @@ static uint32_t janus_pp_h265_eg_decode(uint8_t *base, uint32_t *offset) { while(janus_pp_h265_eg_getbit(base, (*offset)++) == 0) zeros++; uint32_t res = 1 << zeros; - int32_t i = 0; - for(i=zeros-1; i>=0; i--) { - res |= janus_pp_h265_eg_getbit(base, (*offset)++) << i; + if(zeros > 0) { + int32_t i = 0; + for(i=zeros-1; i>=0; i--) { + res |= janus_pp_h265_eg_getbit(base, (*offset)++) << i; + } } return res-1; } diff --git a/postprocessing/pp-webm.c b/postprocessing/pp-webm.c index 2af290af6a..b00bf1739a 100644 --- a/postprocessing/pp-webm.c +++ b/postprocessing/pp-webm.c @@ -547,8 +547,14 @@ int janus_pp_webm_process(FILE *file, janus_pp_frame_packet *list, gboolean vp8, } } /* Frame manipulation */ - memcpy(received_frame + frameLen, buffer, len); - frameLen += len; + if(len > 0) { + if(frameLen + len + AV_INPUT_BUFFER_PADDING_SIZE > numBytes) { + JANUS_LOG(LOG_WARN, "Frame exceeds buffer size...\n"); + } else { + memcpy(received_frame + frameLen, buffer, len); + frameLen += len; + } + } if(len == 0) break; /* Check if timestamp changes: marker bit is not mandatory, and may be lost as well */ diff --git a/transports/janus_http.c b/transports/janus_http.c index 7448c09d7c..1f8dd5f29c 100644 --- a/transports/janus_http.c +++ b/transports/janus_http.c @@ -321,11 +321,15 @@ static void janus_http_allow_address(const char *ip, gboolean admin) { static gboolean janus_http_is_allowed(const char *ip, gboolean admin) { if(ip == NULL) return FALSE; - if(!admin && janus_http_access_list == NULL) + janus_mutex_lock(&access_list_mutex); + if(!admin && janus_http_access_list == NULL) { + janus_mutex_unlock(&access_list_mutex); return TRUE; - if(admin && janus_http_admin_access_list == NULL) + } + if(admin && janus_http_admin_access_list == NULL) { + janus_mutex_unlock(&access_list_mutex); return TRUE; - janus_mutex_lock(&access_list_mutex); + } GList *temp = admin ? janus_http_admin_access_list : janus_http_access_list; while(temp) { const char *allowed = (const char *)temp->data; diff --git a/transports/janus_websockets.c b/transports/janus_websockets.c index 0d1afe9a1b..ef7388ad11 100644 --- a/transports/janus_websockets.c +++ b/transports/janus_websockets.c @@ -346,18 +346,17 @@ static void janus_websockets_allow_address(const char *ip, gboolean admin) { janus_mutex_unlock(&access_list_mutex); } static gboolean janus_websockets_is_allowed(const char *ip, gboolean admin) { - JANUS_LOG(LOG_VERB, "Checking if %s is allowed to contact %s interface\n", ip, admin ? "admin" : "janus"); if(ip == NULL) return FALSE; + janus_mutex_lock(&access_list_mutex); if(!admin && janus_websockets_access_list == NULL) { - JANUS_LOG(LOG_VERB, "Yep\n"); + janus_mutex_unlock(&access_list_mutex); return TRUE; } if(admin && janus_websockets_admin_access_list == NULL) { - JANUS_LOG(LOG_VERB, "Yeah\n"); + janus_mutex_unlock(&access_list_mutex); return TRUE; } - janus_mutex_lock(&access_list_mutex); GList *temp = admin ? janus_websockets_admin_access_list : janus_websockets_access_list; while(temp) { const char *allowed = (const char *)temp->data; diff --git a/turnrest.c b/turnrest.c index db17cd240d..eb5a698952 100644 --- a/turnrest.c +++ b/turnrest.c @@ -164,7 +164,6 @@ janus_turnrest_response *janus_turnrest_request(const char *user) { char request_uri[1024]; g_snprintf(request_uri, 1024, "%s?%s", api_server, query_string); JANUS_LOG(LOG_VERB, "Sending request: %s\n", request_uri); - janus_mutex_unlock(&api_mutex); curl_easy_setopt(curl, CURLOPT_URL, request_uri); curl_easy_setopt(curl, (api_http_get ? CURLOPT_HTTPGET : CURLOPT_POST), 1); if(!api_http_get) { @@ -172,6 +171,7 @@ janus_turnrest_response *janus_turnrest_request(const char *user) { curl_easy_setopt(curl, CURLOPT_POSTFIELDS, query_string); } curl_easy_setopt(curl, CURLOPT_TIMEOUT, api_timeout); + janus_mutex_unlock(&api_mutex); /* For getting data, we use an helper struct and the libcurl callback */ janus_turnrest_buffer data; data.buffer = g_malloc0(1);