Skip to content

Commit 5b15ded

Browse files
authored
Security fixes in SDP code (#2214)
* Fixed NULL pointer dereference in sdp.c * Fixed stack memory leak in janus.c (and regular memory leak in sdp.c) * Fixed NULL pointer dereference in sdp.c (preparse mid) * Fixed buffer overflow when merging SDPs in sdp.c
1 parent 00cb8c4 commit 5b15ded

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

janus.c

+2
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,7 @@ int janus_process_incoming_request(janus_request *request) {
13241324
JANUS_LOG(LOG_VERB, "[%"SCNu64"] Remote SDP:\n%s", handle->handle_id, jsep_sdp);
13251325
/* Is this valid SDP? */
13261326
char error_str[512];
1327+
error_str[0] = '\0';
13271328
int audio = 0, video = 0, data = 0;
13281329
janus_sdp *parsed_sdp = janus_sdp_preparse(handle, jsep_sdp, error_str, sizeof(error_str), &audio, &video, &data);
13291330
if(parsed_sdp == NULL) {
@@ -3368,6 +3369,7 @@ json_t *janus_plugin_handle_sdp(janus_plugin_session *plugin_session, janus_plug
33683369
}
33693370
/* Is this valid SDP? */
33703371
char error_str[512];
3372+
error_str[0] = '\0';
33713373
int audio = 0, video = 0, data = 0;
33723374
janus_sdp *parsed_sdp = janus_sdp_preparse(ice_handle, sdp, error_str, sizeof(error_str), &audio, &video, &data);
33733375
if(parsed_sdp == NULL) {

sdp.c

+11-4
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,16 @@ janus_sdp *janus_sdp_preparse(void *ice_handle, const char *jsep_sdp, char *erro
5959
if(a->name) {
6060
if(!strcasecmp(a->name, "mid")) {
6161
/* Found mid attribute */
62+
if(a->value == NULL) {
63+
JANUS_LOG(LOG_ERR, "[%"SCNu64"] Invalid mid attribute (no value)\n", handle->handle_id);
64+
janus_sdp_destroy(parsed_sdp);
65+
return NULL;
66+
}
6267
if(m->type == JANUS_SDP_AUDIO && m->port > 0) {
6368
JANUS_LOG(LOG_VERB, "[%"SCNu64"] Audio mid: %s\n", handle->handle_id, a->value);
6469
if(strlen(a->value) > 16) {
6570
JANUS_LOG(LOG_ERR, "[%"SCNu64"] Audio mid too large: (%zu > 16)\n", handle->handle_id, strlen(a->value));
71+
janus_sdp_destroy(parsed_sdp);
6672
return NULL;
6773
}
6874
if(handle->audio_mid == NULL)
@@ -73,6 +79,7 @@ janus_sdp *janus_sdp_preparse(void *ice_handle, const char *jsep_sdp, char *erro
7379
JANUS_LOG(LOG_VERB, "[%"SCNu64"] Video mid: %s\n", handle->handle_id, a->value);
7480
if(strlen(a->value) > 16) {
7581
JANUS_LOG(LOG_ERR, "[%"SCNu64"] Video mid too large: (%zu > 16)\n", handle->handle_id, strlen(a->value));
82+
janus_sdp_destroy(parsed_sdp);
7683
return NULL;
7784
}
7885
if(handle->video_mid == NULL)
@@ -114,7 +121,7 @@ int janus_sdp_process(void *ice_handle, janus_sdp *remote_sdp, gboolean update)
114121
GList *temp = remote_sdp->attributes;
115122
while(temp) {
116123
janus_sdp_attribute *a = (janus_sdp_attribute *)temp->data;
117-
if(a && a->name) {
124+
if(a && a->name && a->value) {
118125
if(!strcasecmp(a->name, "fingerprint")) {
119126
JANUS_LOG(LOG_VERB, "[%"SCNu64"] Fingerprint (global) : %s\n", handle->handle_id, a->value);
120127
if(strcasestr(a->value, "sha-256 ") == a->value) {
@@ -1229,14 +1236,14 @@ char *janus_sdp_merge(void *ice_handle, janus_sdp *anon, gboolean offer) {
12291236
if(audio == 1) {
12301237
g_snprintf(buffer_part, sizeof(buffer_part),
12311238
" %s", handle->audio_mid ? handle->audio_mid : "audio");
1232-
g_strlcat(buffer, buffer_part, JANUS_BUFSIZE);
1239+
g_strlcat(buffer, buffer_part, sizeof(buffer));
12331240
}
12341241
} else if(m->type == JANUS_SDP_VIDEO) {
12351242
video++;
12361243
if(video == 1) {
12371244
g_snprintf(buffer_part, sizeof(buffer_part),
12381245
" %s", handle->video_mid ? handle->video_mid : "video");
1239-
g_strlcat(buffer, buffer_part, JANUS_BUFSIZE);
1246+
g_strlcat(buffer, buffer_part, sizeof(buffer));
12401247
}
12411248
#ifdef HAVE_SCTP
12421249
} else if(m->type == JANUS_SDP_APPLICATION) {
@@ -1245,7 +1252,7 @@ char *janus_sdp_merge(void *ice_handle, janus_sdp *anon, gboolean offer) {
12451252
if(data == 1) {
12461253
g_snprintf(buffer_part, sizeof(buffer_part),
12471254
" %s", handle->data_mid ? handle->data_mid : "data");
1248-
g_strlcat(buffer, buffer_part, JANUS_BUFSIZE);
1255+
g_strlcat(buffer, buffer_part, sizeof(buffer));
12491256
}
12501257
#endif
12511258
}

0 commit comments

Comments
 (0)