From b9a4ab8c8c00c8d53584d6f7e31729b5027c8dd6 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Thu, 15 Jun 2017 12:33:22 -0500 Subject: [PATCH] chan_pjsip: Fix PJSIP_MEDIA_OFFER dialplan function read. The construction of the returned string assumed incorrectly that the supplied buffer would always be initialized as an empty string. If it is not an empty string we could overrun the supplied buffer by the length of the non-empty buffer string plus one. It is also theoreticaly possible for the supplied buffer to be overrun by a string terminator during a read operation even if the supplied buffer is an empty string. * Fix the assumption that the supplied buffer would already be an empty string. The buffer is not guaranteed to contain an empty string by all possible callers. * Fix string terminator buffer overrun potential. Change-Id: If6a0806806527678c8554b1dcb34fd7808aa95c9 --- channels/pjsip/dialplan_functions.c | 35 +++++++++++++++++------------ 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/channels/pjsip/dialplan_functions.c b/channels/pjsip/dialplan_functions.c index d11636ccc4..22d773820b 100644 --- a/channels/pjsip/dialplan_functions.c +++ b/channels/pjsip/dialplan_functions.c @@ -929,36 +929,40 @@ int pjsip_acf_dial_contacts_read(struct ast_channel *chan, const char *cmd, char static int media_offer_read_av(struct ast_sip_session *session, char *buf, size_t len, enum ast_media_type media_type) { - int i, size = 0; + int idx; + size_t accum = 0; - for (i = 0; i < ast_format_cap_count(session->req_caps); i++) { - struct ast_format *fmt = ast_format_cap_get_format(session->req_caps, i); + /* Note: buf is not terminated while the string is being built. */ + for (idx = 0; idx < ast_format_cap_count(session->req_caps); ++idx) { + struct ast_format *fmt; + size_t size; + fmt = ast_format_cap_get_format(session->req_caps, idx); if (ast_format_get_type(fmt) != media_type) { ao2_ref(fmt, -1); continue; } - /* add one since we'll include a comma */ + /* Add one for a comma or terminator */ size = strlen(ast_format_get_name(fmt)) + 1; if (len < size) { ao2_ref(fmt, -1); break; } - len -= size; - - /* no reason to use strncat here since we have already ensured buf has - enough space, so strcat can be safely used */ - strcat(buf, ast_format_get_name(fmt)); - strcat(buf, ","); + /* Append the format name */ + strcpy(buf + accum, ast_format_get_name(fmt));/* Safe */ ao2_ref(fmt, -1); - } - if (size) { - /* remove the extra comma */ - buf[strlen(buf) - 1] = '\0'; + accum += size; + len -= size; + + /* The last comma on the built string will be set to the terminator. */ + buf[accum - 1] = ','; } + + /* Remove the trailing comma or terminate an empty buffer. */ + buf[accum ? accum - 1 : 0] = '\0'; return 0; } @@ -998,6 +1002,9 @@ int pjsip_acf_media_offer_read(struct ast_channel *chan, const char *cmd, char * return media_offer_read_av(channel->session, buf, len, AST_MEDIA_TYPE_AUDIO); } else if (!strcmp(data, "video")) { return media_offer_read_av(channel->session, buf, len, AST_MEDIA_TYPE_VIDEO); + } else { + /* Ensure that the buffer is empty */ + buf[0] = '\0'; } return 0;