[Asterisk-code-review] chan sip: bigger buffers for headers, better failure mode (asterisk[master])
Vasil Kolev
asteriskteam at digium.com
Tue May 31 09:11:12 CDT 2016
Vasil Kolev has uploaded a new change for review.
https://gerrit.asterisk.org/2923
Change subject: chan_sip: bigger buffers for headers, better failure mode
......................................................................
chan_sip: bigger buffers for headers, better failure mode
Currently chan_sip can give weird messages if the contacts don't
fit in the From: or To: messages. This fix changes the from and
to variables to use ast_str, allocates and deallocates them and
resizes them if needed up to SIPHEADERMAX.
ASTERISK-26069
Change-Id: I1b68fcbddca6f6cc7d7a92fe1cb0d5430282b2b3
---
M channels/chan_sip.c
1 file changed, 50 insertions(+), 29 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/23/2923/1
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 19f8aa3..b26ef32 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -14155,9 +14155,11 @@
/*! \brief Initiate new SIP request to peer/user */
static void initreqprep(struct sip_request *req, struct sip_pvt *p, int sipmethod, const char * const explicit_uri)
{
- struct ast_str *invite = ast_str_alloca(256);
- char from[256];
- char to[256];
+#define SIPHEADER 256
+#define SIPHEADERMAX 1024
+ struct ast_str *invite = ast_str_create(SIPHEADER);
+ struct ast_str *from = ast_str_create(SIPHEADER);
+ struct ast_str *to = ast_str_create(SIPHEADER);
char tmp_n[SIPBUFSIZE/2]; /* build a local copy of 'n' if needed */
char tmp_l[SIPBUFSIZE/2]; /* build a local copy of 'l' if needed */
const char *l = NULL; /* XXX what is this, exactly ? */
@@ -14259,19 +14261,25 @@
ourport = (p->fromdomainport && (p->fromdomainport != STANDARD_SIP_PORT)) ? p->fromdomainport : ast_sockaddr_port(&p->ourip);
if (!sip_standard_port(p->socket.type, ourport)) {
- ret = snprintf(from, sizeof(from), "<sip:%s@%s:%d>;tag=%s", tmp_l, d, ourport, p->tag);
+ ret = ast_str_set(&from, SIPHEADERMAX, "<sip:%s@%s:%d>;tag=%s", tmp_l, d, ourport, p->tag);
} else {
- ret = snprintf(from, sizeof(from), "<sip:%s@%s>;tag=%s", tmp_l, d, p->tag);
+ ret = ast_str_set(&from, SIPHEADERMAX, "<sip:%s@%s>;tag=%s", tmp_l, d, p->tag);
}
- if (ret < 0 || ret >= sizeof(from)) { /* a return value of size or more means that the output was truncated */
+ if (ret < 0 || ret >= SIPHEADERMAX) { /* a return value of size or more means that the output was truncated */
/* We don't have an escape path from here... */
ast_log(LOG_ERROR, "The From header was truncated in call '%s'. This call setup will fail.\n", p->callid);
+ /* XXX Make sure the field is terminated with >, to make sure we don't actually create a totally broken packet
+ See https://issues.asterisk.org/jira/browse/ASTERISK-26069
+ */
+ ast_str_append(&from, SIPHEADERMAX+1, ">");
+
}
/* If a caller id name was specified, prefix a display name, if there is enough room. */
if (cid_has_name || !cid_has_num) {
- size_t written = strlen(from);
- ssize_t left = sizeof(from) - written - 4; /* '"" \0' */
+ char *from_buf = ast_str_buffer(from);
+ size_t written = strlen(from_buf);
+ ssize_t left = SIPHEADERMAX - written - 4; /* '"" \0' */
if (left > 0) {
size_t name_len;
if (sip_cfg.pedanticsipchecking) {
@@ -14282,43 +14290,48 @@
if (left < name_len) {
name_len = left;
}
- memmove(from + name_len + 3, from, written + 1);
- from[0] = '"';
- memcpy(from + 1, n, name_len);
- from[name_len + 1] = '"';
- from[name_len + 2] = ' ';
+ ast_str_make_space(&from, name_len + written + 4);
+
+ /* needed again, as ast_str_make_space coud've changed the pointer */
+ from_buf = ast_str_buffer(from);
+
+ memmove(from_buf + name_len + 3, from_buf, written + 1);
+ from_buf[0] = '"';
+ memcpy(from_buf + 1, n, name_len);
+ from_buf[name_len + 1] = '"';
+ from_buf[name_len + 2] = ' ';
}
}
if (!ast_strlen_zero(explicit_uri)) {
- ast_str_set(&invite, 0, "%s", explicit_uri);
+ ast_str_set(&invite, SIPHEADERMAX, "%s", explicit_uri);
} else {
/* If we're calling a registered SIP peer, use the fullcontact to dial to the peer */
if (!ast_strlen_zero(p->fullcontact)) {
/* If we have full contact, trust it */
- ast_str_append(&invite, 0, "%s", p->fullcontact);
+ ast_str_append(&invite, SIPHEADERMAX, "%s", p->fullcontact);
} else {
/* Otherwise, use the username while waiting for registration */
- ast_str_append(&invite, 0, "sip:");
+ ast_str_append(&invite, SIPHEADERMAX, "sip:");
if (!ast_strlen_zero(p->username)) {
n = p->username;
if (sip_cfg.pedanticsipchecking) {
ast_uri_encode(n, tmp_n, sizeof(tmp_n), ast_uri_sip_user);
n = tmp_n;
}
- ast_str_append(&invite, 0, "%s@", n);
+ ast_str_append(&invite, SIPHEADERMAX, "%s@", n);
}
- ast_str_append(&invite, 0, "%s", p->tohost);
+ ast_str_append(&invite, SIPHEADERMAX, "%s", p->tohost);
if (p->portinuri) {
- ast_str_append(&invite, 0, ":%d", ast_sockaddr_port(&p->sa));
+ ast_str_append(&invite, SIPHEADERMAX, ":%d", ast_sockaddr_port(&p->sa));
}
- ast_str_append(&invite, 0, "%s", urioptions);
+ ast_str_append(&invite, SIPHEADERMAX, "%s", urioptions);
}
}
/* If custom URI options have been provided, append them */
if (p->options && !ast_strlen_zero(p->options->uri_options))
- ast_str_append(&invite, 0, ";%s", p->options->uri_options);
+ ast_str_append(&invite, SIPHEADERMAX, ";%s", p->options->uri_options);
/* This is the request URI, which is the next hop of the call
which may or may not be the destination of the call
@@ -14329,24 +14342,28 @@
/*! \todo Need to add back the VXML URL here at some point, possibly use build_string for all this junk */
if (!strchr(p->todnid, '@')) {
/* We have no domain in the dnid */
- ret = snprintf(to, sizeof(to), "<sip:%s@%s>%s%s", p->todnid, p->tohost, ast_strlen_zero(p->theirtag) ? "" : ";tag=", p->theirtag);
+ ret = ast_str_set(&to, SIPHEADERMAX, "<sip:%s@%s>%s%s", p->todnid, p->tohost, ast_strlen_zero(p->theirtag) ? "" : ";tag=", p->theirtag);
} else {
- ret = snprintf(to, sizeof(to), "<sip:%s>%s%s", p->todnid, ast_strlen_zero(p->theirtag) ? "" : ";tag=", p->theirtag);
+ ret = ast_str_set(&to, SIPHEADERMAX, "<sip:%s>%s%s", p->todnid, ast_strlen_zero(p->theirtag) ? "" : ";tag=", p->theirtag);
}
} else {
if (sipmethod == SIP_NOTIFY && !ast_strlen_zero(p->theirtag)) {
/* If this is a NOTIFY, use the From: tag in the subscribe (RFC 3265) */
- ret = snprintf(to, sizeof(to), "<%s%s>;tag=%s", (strncasecmp(p->uri, "sip:", 4) ? "sip:" : ""), p->uri, p->theirtag);
+ ret = ast_str_set(&to, SIPHEADERMAX, "<%s%s>;tag=%s", (strncasecmp(p->uri, "sip:", 4) ? "sip:" : ""), p->uri, p->theirtag);
} else if (p->options && p->options->vxml_url) {
/* If there is a VXML URL append it to the SIP URL */
- ret = snprintf(to, sizeof(to), "<%s>;%s", p->uri, p->options->vxml_url);
+ ret = ast_str_set(&to, SIPHEADERMAX, "<%s>;%s", p->uri, p->options->vxml_url);
} else {
- ret = snprintf(to, sizeof(to), "<%s>", p->uri);
+ ret = ast_str_set(&to, SIPHEADERMAX, "<%s>", p->uri);
}
}
- if (ret < 0 || ret >= sizeof(to)) { /* a return value of size or more means that the output was truncated */
+ if (ret < 0 || ret >= SIPHEADERMAX) { /* a return value of size or more means that the output was truncated */
/* We don't have an escape path from here... */
ast_log(LOG_ERROR, "The To header was truncated in call '%s'. This call setup will fail.\n", p->callid);
+ /* XXX Make sure the field is terminated with >, to make sure we don't actually create a totally broken packet
+ See https://issues.asterisk.org/jira/browse/ASTERISK-26069
+ */
+ ast_str_append(&to, SIPHEADERMAX+1, ">");
}
init_req(req, sipmethod, p->uri);
@@ -14361,8 +14378,8 @@
*/
add_route(req, &p->route, 0);
- add_header(req, "From", from);
- add_header(req, "To", to);
+ add_header(req, "From", ast_str_buffer(from));
+ add_header(req, "To", ast_str_buffer(to));
ast_string_field_set(p, exten, l);
build_contact(p, req, 0);
add_header(req, "Contact", p->our_contact);
@@ -14371,6 +14388,10 @@
if (!ast_strlen_zero(global_useragent)) {
add_header(req, "User-Agent", global_useragent);
}
+
+ free(from);
+ free(to);
+ free(invite);
}
/*! \brief Add "Diversion" header to outgoing message
--
To view, visit https://gerrit.asterisk.org/2923
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1b68fcbddca6f6cc7d7a92fe1cb0d5430282b2b3
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Vasil Kolev <vasil.kolev at securax.org>
More information about the asterisk-code-review
mailing list