[asterisk-commits] mjordan: branch 1.8 r378269 - in /branches/1.8: channels/ channels/sip/includ...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Jan 2 09:16:15 CST 2013
Author: mjordan
Date: Wed Jan 2 09:16:10 2013
New Revision: 378269
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=378269
Log:
Resolve crashes due to large stack allocations when using TCP
Asterisk had several places where messages received over various network
transports may be copied in a single stack allocation. In the case of TCP,
since multiple packets in a stream may be concatenated together, this can
lead to large allocations that overflow the stack.
This patch modifies those portions of Asterisk using TCP to either
favor heap allocations or use an upper bound to ensure that the stack will not
overflow:
* For SIP, the allocation now has an upper limit
* For HTTP, the allocation is now a heap allocation instead of a stack
allocation
* For XMPP (in res_jabber), the allocation has been eliminated since it was
unnecesary.
Note that the HTTP portion of this issue was independently found by Brandon
Edwards of Exodus Intelligence.
(issue ASTERISK-20658)
Reported by: wdoekes, Brandon Edwards
Tested by: mmichelson, wdoekes
patches:
ASTERISK-20658_res_jabber.c.patch uploaded by mmichelson (license 5049)
issueA20658_http_postvars_use_malloc2.patch uploaded by wdoekes (license 5674)
issueA20658_limit_sip_packet_size3.patch uploaded by wdoekes (license 5674)
Modified:
branches/1.8/channels/chan_sip.c
branches/1.8/channels/sip/include/sip.h
branches/1.8/main/http.c
branches/1.8/res/res_jabber.c
Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=378269&r1=378268&r2=378269
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Wed Jan 2 09:16:10 2013
@@ -2524,19 +2524,20 @@
int authenticated, time_t start, struct sip_threadinfo *me)
{
int res, content_length, after_poll = 1, need_poll = 1;
+ size_t datalen = ast_str_strlen(req->data);
char buf[1024] = "";
int timeout = -1;
-
- /* Read in headers one line at a time */
- while (ast_str_strlen(req->data) < 4 || strncmp(REQ_OFFSET_TO_STR(req, data->used - 4), "\r\n\r\n", 4)) {
- if (!tcptls_session->client && !authenticated) {
- if ((timeout = sip_check_authtimeout(start)) < 0) {
- ast_debug(2, "SIP SSL server failed to determine authentication timeout\n");
+
+ /* Read in headers one line at a time */
+ while (datalen < 4 || strncmp(REQ_OFFSET_TO_STR(req, data->used - 4), "\r\n\r\n", 4)) {
+ if (!tcptls_session->client && !authenticated) {
+ if ((timeout = sip_check_authtimeout(start)) < 0) {
+ ast_debug(2, "SIP TLS server failed to determine authentication timeout\n");
return -1;
}
if (timeout == 0) {
- ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "SSL": "TCP");
+ ast_debug(2, "SIP TLS server timed out\n");
return -1;
}
} else {
@@ -2551,11 +2552,11 @@
after_poll = 1;
res = ast_wait_for_input(tcptls_session->fd, timeout);
if (res < 0) {
- ast_debug(2, "SIP TCP server :: ast_wait_for_input returned %d\n", res);
+ ast_debug(2, "SIP TLS server :: ast_wait_for_input returned %d\n", res);
return -1;
} else if (res == 0) {
/* timeout */
- ast_debug(2, "SIP TCP server timed out\n");
+ ast_debug(2, "SIP TLS server timed out\n");
return -1;
}
}
@@ -2576,6 +2577,13 @@
return -1;
}
ast_str_append(&req->data, 0, "%s", buf);
+
+ datalen = ast_str_strlen(req->data);
+ if (datalen > SIP_MAX_PACKET_SIZE) {
+ ast_log(LOG_WARNING, "Rejecting TLS packet from '%s' because way too large: %zu\n",
+ ast_sockaddr_stringify(&tcptls_session->remote_address), datalen);
+ return -1;
+ }
}
copy_request(reqcpy, req);
parse_request(reqcpy);
@@ -2589,7 +2597,7 @@
}
if (timeout == 0) {
- ast_debug(2, "SIP SSL server timed out\n");
+ ast_debug(2, "SIP TLS server timed out\n");
return -1;
}
} else {
@@ -2601,11 +2609,11 @@
after_poll = 1;
res = ast_wait_for_input(tcptls_session->fd, timeout);
if (res < 0) {
- ast_debug(2, "SIP TCP server :: ast_wait_for_input returned %d\n", res);
+ ast_debug(2, "SIP TLS server :: ast_wait_for_input returned %d\n", res);
return -1;
} else if (res == 0) {
/* timeout */
- ast_debug(2, "SIP TCP server timed out\n");
+ ast_debug(2, "SIP TLS server timed out\n");
return -1;
}
}
@@ -2628,6 +2636,13 @@
}
content_length -= strlen(buf);
ast_str_append(&req->data, 0, "%s", buf);
+
+ datalen = ast_str_strlen(req->data);
+ if (datalen > SIP_MAX_PACKET_SIZE) {
+ ast_log(LOG_WARNING, "Rejecting TLS packet from '%s' because way too large: %zu\n",
+ ast_sockaddr_stringify(&tcptls_session->remote_address), datalen);
+ return -1;
+ }
}
}
/*! \todo XXX If there's no Content-Length or if the content-length and what
@@ -2801,6 +2816,8 @@
enum message_integrity message_integrity = MESSAGE_FRAGMENT;
while (message_integrity == MESSAGE_FRAGMENT) {
+ size_t datalen;
+
if (ast_str_strlen(tcptls_session->overflow_buf) == 0) {
char readbuf[4097];
int timeout;
@@ -2839,6 +2856,13 @@
} else {
ast_str_append(&req->data, 0, "%s", ast_str_buffer(tcptls_session->overflow_buf));
ast_str_reset(tcptls_session->overflow_buf);
+ }
+
+ datalen = ast_str_strlen(req->data);
+ if (datalen > SIP_MAX_PACKET_SIZE) {
+ ast_log(LOG_WARNING, "Rejecting TCP packet from '%s' because way too large: %zu\n",
+ ast_sockaddr_stringify(&tcptls_session->remote_address), datalen);
+ return -1;
}
message_integrity = check_message_integrity(&req->data, &tcptls_session->overflow_buf);
@@ -2911,7 +2935,7 @@
}
me->threadid = pthread_self();
- ast_debug(2, "Starting thread for %s server\n", tcptls_session->ssl ? "SSL" : "TCP");
+ ast_debug(2, "Starting thread for %s server\n", tcptls_session->ssl ? "TLS" : "TCP");
/* set up pollfd to watch for reads on both the socket and the alert_pipe */
fds[0].fd = tcptls_session->fd;
@@ -2945,7 +2969,7 @@
}
if (timeout == 0) {
- ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "SSL": "TCP");
+ ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "TLS": "TCP");
goto cleanup;
}
} else {
@@ -2955,11 +2979,11 @@
if (ast_str_strlen(tcptls_session->overflow_buf) == 0) {
res = ast_poll(fds, 2, timeout); /* polls for both socket and alert_pipe */
if (res < 0) {
- ast_debug(2, "SIP %s server :: ast_wait_for_input returned %d\n", tcptls_session->ssl ? "SSL": "TCP", res);
+ ast_debug(2, "SIP %s server :: ast_wait_for_input returned %d\n", tcptls_session->ssl ? "TLS": "TCP", res);
goto cleanup;
} else if (res == 0) {
/* timeout */
- ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "SSL": "TCP");
+ ast_debug(2, "SIP %s server timed out\n", tcptls_session->ssl ? "TLS": "TCP");
goto cleanup;
}
}
@@ -3041,7 +3065,7 @@
}
}
- ast_debug(2, "Shutting down thread for %s server\n", tcptls_session->ssl ? "SSL" : "TCP");
+ ast_debug(2, "Shutting down thread for %s server\n", tcptls_session->ssl ? "TLS" : "TCP");
cleanup:
if (tcptls_session && !tcptls_session->client && !authenticated) {
Modified: branches/1.8/channels/sip/include/sip.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/sip/include/sip.h?view=diff&rev=378269&r1=378268&r2=378269
==============================================================================
--- branches/1.8/channels/sip/include/sip.h (original)
+++ branches/1.8/channels/sip/include/sip.h Wed Jan 2 09:16:10 2013
@@ -96,6 +96,7 @@
#define SIP_MAX_HEADERS 64 /*!< Max amount of SIP headers to read */
#define SIP_MAX_LINES 256 /*!< Max amount of lines in SIP attachment (like SDP) */
+#define SIP_MAX_PACKET_SIZE 20480 /*!< Max SIP packet size */
#define SIP_MIN_PACKET 4096 /*!< Initialize size of memory to allocate for packets */
#define MAX_HISTORY_ENTRIES 50 /*!< Max entires in the history list for a sip_pvt */
Modified: branches/1.8/main/http.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/http.c?view=diff&rev=378269&r1=378268&r2=378269
==============================================================================
--- branches/1.8/main/http.c (original)
+++ branches/1.8/main/http.c Wed Jan 2 09:16:10 2013
@@ -622,6 +622,7 @@
int content_length = 0;
struct ast_variable *v, *post_vars=NULL, *prev = NULL;
char *buf, *var, *val;
+ int res;
for (v = headers; v; v = v->next) {
if (!strcasecmp(v->name, "Content-Type")) {
@@ -634,19 +635,27 @@
for (v = headers; v; v = v->next) {
if (!strcasecmp(v->name, "Content-Length")) {
- content_length = atoi(v->value) + 1;
+ content_length = atoi(v->value);
break;
}
}
- if (!content_length) {
+ if (content_length <= 0) {
return NULL;
}
- buf = ast_alloca(content_length);
- if (!fgets(buf, content_length, ser->f)) {
+ buf = ast_malloc(content_length + 1);
+ if (!buf) {
return NULL;
}
+
+ res = fread(buf, 1, content_length, ser->f);
+ if (res < content_length) {
+ /* Error, distinguishable by ferror() or feof(), but neither
+ * is good. */
+ goto done;
+ }
+ buf[content_length] = '\0';
while ((val = strsep(&buf, "&"))) {
var = strsep(&val, "=");
@@ -665,6 +674,9 @@
prev = v;
}
}
+
+done:
+ ast_free(buf);
return post_vars;
}
Modified: branches/1.8/res/res_jabber.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_jabber.c?view=diff&rev=378269&r1=378268&r2=378269
==============================================================================
--- branches/1.8/res/res_jabber.c (original)
+++ branches/1.8/res/res_jabber.c Wed Jan 2 09:16:10 2013
@@ -766,7 +766,7 @@
*/
static int acf_jabberreceive_read(struct ast_channel *chan, const char *name, char *data, char *buf, size_t buflen)
{
- char *aux = NULL, *parse = NULL;
+ char *parse = NULL;
int timeout;
int jidlen, resourcelen;
struct timeval start;
@@ -883,7 +883,7 @@
continue;
}
found = 1;
- aux = ast_strdupa(tmp->message);
+ ast_copy_string(buf, tmp->message, buflen);
AST_LIST_REMOVE_CURRENT(list);
aji_message_destroy(tmp);
break;
@@ -908,7 +908,6 @@
ast_log(LOG_NOTICE, "Timed out : no message received from %s\n", args.jid);
return -1;
}
- ast_copy_string(buf, aux, buflen);
return 0;
}
More information about the asterisk-commits
mailing list