diff --git a/ChangeLog b/ChangeLog index af0dd05077..287d52e758 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,38 @@ +2015-01-28 Asterisk Development Team + + * Asterisk 13.1.1 Released. + + * Mitigate possible HTTP injection attacks using CURL() function in + Asterisk. + + CVE-2014-8150 disclosed a vulnerability in libcURL where HTTP request + injection can be performed given properly-crafted URLs. + + Since Asterisk makes use of libcURL, and it is possible that users of + Asterisk may get cURL URLs from user input or remote sources, we have + made a patch to Asterisk to prevent such HTTP injection attacks from + originating from Asterisk. + + ASTERISK-24676 #close + Reported by: Matt Jordan, Olle Johansson + + Review: https://reviewboard.asterisk.org/r/4364 + + AST-2015-002 + + * Fix file descriptor leak in RTP code. + + SIP requests that offered codecs incompatible with configured values + could result in the allocation of RTP and RTCP ports that would not + get reclaimed later. + + ASTERISK-24666 #close + Reported by Y Ateya + + Review: https://reviewboard.asterisk.org/r/4323 + + AST-2015-001 + 2014-12-15 Asterisk Development Team * Asterisk 13.1.0 Released. diff --git a/funcs/func_curl.c b/funcs/func_curl.c index 22ee5821cd..3ecd3ff7fa 100644 --- a/funcs/func_curl.c +++ b/funcs/func_curl.c @@ -50,6 +50,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #include "asterisk/app.h" #include "asterisk/utils.h" #include "asterisk/threadstorage.h" +#include "asterisk/test.h" /*** DOCUMENTATION @@ -567,6 +568,31 @@ static void curl_instance_cleanup(void *data) AST_THREADSTORAGE_CUSTOM(curl_instance, curl_instance_init, curl_instance_cleanup); AST_THREADSTORAGE(thread_escapebuf); +/*! + * \brief Check for potential HTTP injection risk. + * + * CVE-2014-8150 brought up the fact that HTTP proxies are subject to injection + * attacks. An HTTP URL sent to a proxy contains a carriage-return linefeed combination, + * followed by a complete HTTP request. Proxies will handle this as two separate HTTP + * requests rather than as a malformed URL. + * + * libcURL patched this vulnerability in version 7.40.0, but we have no guarantee that + * Asterisk systems will be using an up-to-date cURL library. Therefore, we implement + * the same fix as libcURL for determining if a URL is vulnerable to an injection attack. + * + * \param url The URL to check for vulnerability + * \retval 0 The URL is not vulnerable + * \retval 1 The URL is vulnerable. + */ +static int url_is_vulnerable(const char *url) +{ + if (strpbrk(url, "\r\n")) { + return 1; + } + + return 0; +} + static int acf_curl_helper(struct ast_channel *chan, const char *cmd, char *info, char *buf, struct ast_str **input_str, ssize_t len) { struct ast_str *escapebuf = ast_str_thread_get(&thread_escapebuf, 16); @@ -604,6 +630,11 @@ static int acf_curl_helper(struct ast_channel *chan, const char *cmd, char *info AST_STANDARD_APP_ARGS(args, info); + if (url_is_vulnerable(args.url)) { + ast_log(LOG_ERROR, "URL '%s' is vulnerable to HTTP injection attacks. Aborting CURL() call.\n", args.url); + return -1; + } + if (chan) { ast_autoservice_start(chan); } @@ -762,6 +793,54 @@ static struct ast_custom_function acf_curlopt = { .write = acf_curlopt_write, }; +AST_TEST_DEFINE(vulnerable_url) +{ + const char *bad_urls [] = { + "http://example.com\r\nDELETE http://example.com/everything", + "http://example.com\rDELETE http://example.com/everything", + "http://example.com\nDELETE http://example.com/everything", + "\r\nhttp://example.com", + "\rhttp://example.com", + "\nhttp://example.com", + "http://example.com\r\n", + "http://example.com\r", + "http://example.com\n", + }; + const char *good_urls [] = { + "http://example.com", + "http://example.com/%5Cr%5Cn", + }; + int i; + enum ast_test_result_state res = AST_TEST_PASS; + + switch (cmd) { + case TEST_INIT: + info->name = "vulnerable_url"; + info->category = "/funcs/func_curl/"; + info->summary = "cURL vulnerable URL test"; + info->description = + "Ensure that any combination of '\\r' or '\\n' in a URL invalidates the URL"; + case TEST_EXECUTE: + break; + } + + for (i = 0; i < ARRAY_LEN(bad_urls); ++i) { + if (!url_is_vulnerable(bad_urls[i])) { + ast_test_status_update(test, "String '%s' detected as valid when it should be invalid\n", bad_urls[i]); + res = AST_TEST_FAIL; + } + } + + for (i = 0; i < ARRAY_LEN(good_urls); ++i) { + if (url_is_vulnerable(good_urls[i])) { + ast_test_status_update(test, "String '%s' detected as invalid when it should be valid\n", good_urls[i]); + res = AST_TEST_FAIL; + } + } + + return res; +} + static int unload_module(void) { int res; @@ -769,6 +848,8 @@ static int unload_module(void) res = ast_custom_function_unregister(&acf_curl); res |= ast_custom_function_unregister(&acf_curlopt); + AST_TEST_UNREGISTER(vulnerable_url); + return res; } @@ -786,6 +867,8 @@ static int load_module(void) res = ast_custom_function_register(&acf_curl); res |= ast_custom_function_register(&acf_curlopt); + AST_TEST_REGISTER(vulnerable_url); + return res; } diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c index 7947ce1fce..f96540d1de 100644 --- a/res/res_pjsip_sdp_rtp.c +++ b/res/res_pjsip_sdp_rtp.c @@ -1237,6 +1237,7 @@ static void stream_destroy(struct ast_sip_session_media *session_media) ast_rtp_instance_stop(session_media->rtp); ast_rtp_instance_destroy(session_media->rtp); } + session_media->rtp = NULL; } /*! \brief SDP handler for 'audio' media stream */ diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c index f337325122..4467203247 100644 --- a/res/res_pjsip_session.c +++ b/res/res_pjsip_session.c @@ -186,6 +186,26 @@ void ast_sip_session_unregister_sdp_handler(struct ast_sip_session_sdp_handler * ao2_callback_data(sdp_handlers, OBJ_KEY | OBJ_UNLINK | OBJ_NODATA, remove_handler, (void *)stream_type, handler); } +/*! + * \brief Set an SDP stream handler for a corresponding session media. + * + * \note Always use this function to set the SDP handler for a session media. + * + * This function will properly free resources on the SDP handler currently being + * used by the session media, then set the session media to use the new SDP + * handler. + */ +static void session_media_set_handler(struct ast_sip_session_media *session_media, + struct ast_sip_session_sdp_handler *handler) +{ + ast_assert(session_media->handler != handler); + + if (session_media->handler) { + session_media->handler->stream_destroy(session_media); + } + session_media->handler = handler; +} + static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sdp_session *sdp) { int i; @@ -235,6 +255,9 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd continue; } AST_LIST_TRAVERSE(&handler_list->list, handler, next) { + if (handler == session_media->handler) { + continue; + } ast_debug(1, "Negotiating incoming SDP media stream '%s' using %s SDP handler\n", session_media->stream_type, handler->id); @@ -249,7 +272,7 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd session_media->stream_type, handler->id); /* Handled by this handler. Move to the next stream */ - session_media->handler = handler; + session_media_set_handler(session_media, handler); handled = 1; break; } @@ -317,6 +340,9 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags) continue; } AST_LIST_TRAVERSE(&handler_list->list, handler, next) { + if (handler == session_media->handler) { + continue; + } ast_debug(1, "Applying negotiated SDP media stream '%s' using %s SDP handler\n", session_media->stream_type, handler->id); @@ -331,7 +357,7 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags) session_media->stream_type, handler->id); /* Handled by this handler. Move to the next stream */ - session_media->handler = handler; + session_media_set_handler(session_media, handler); return CMP_MATCH; } } @@ -742,6 +768,9 @@ static int sdp_requires_deferral(struct ast_sip_session *session, const pjmedia_ continue; } AST_LIST_TRAVERSE(&handler_list->list, handler, next) { + if (handler == session_media->handler) { + continue; + } if (!handler->defer_incoming_sdp_stream) { continue; } @@ -751,15 +780,15 @@ static int sdp_requires_deferral(struct ast_sip_session *session, const pjmedia_ case AST_SIP_SESSION_SDP_DEFER_NOT_HANDLED: continue; case AST_SIP_SESSION_SDP_DEFER_ERROR: - session_media->handler = handler; + session_media_set_handler(session_media, handler); return 0; case AST_SIP_SESSION_SDP_DEFER_NOT_NEEDED: /* Handled by this handler. */ - session_media->handler = handler; + session_media_set_handler(session_media, handler); break; case AST_SIP_SESSION_SDP_DEFER_NEEDED: /* Handled by this handler. */ - session_media->handler = handler; + session_media_set_handler(session_media, handler); return 1; } /* Move to the next stream */ @@ -920,9 +949,21 @@ static int datastore_cmp(void *obj, void *arg, int flags) static void session_media_dtor(void *obj) { struct ast_sip_session_media *session_media = obj; - if (session_media->handler) { - session_media->handler->stream_destroy(session_media); + struct sdp_handler_list *handler_list; + /* It is possible for SDP handlers to allocate memory on a session_media but + * not end up getting set as the handler for this session_media. This traversal + * ensures that all memory allocated by SDP handlers on the session_media is + * cleared (as well as file descriptors, etc.). + */ + handler_list = ao2_find(sdp_handlers, session_media->stream_type, OBJ_KEY); + if (handler_list) { + struct ast_sip_session_sdp_handler *handler; + + AST_LIST_TRAVERSE(&handler_list->list, handler, next) { + handler->stream_destroy(session_media); + } } + ao2_cleanup(handler_list); if (session_media->srtp) { ast_sdp_srtp_destroy(session_media->srtp); } @@ -2054,6 +2095,9 @@ static int add_sdp_streams(void *obj, void *arg, void *data, int flags) /* no handler for this stream type and we have a list to search */ AST_LIST_TRAVERSE(&handler_list->list, handler, next) { + if (handler == session_media->handler) { + continue; + } res = handler->create_outgoing_sdp_stream(session, session_media, answer); if (res < 0) { /* catastrophic error */ @@ -2061,7 +2105,7 @@ static int add_sdp_streams(void *obj, void *arg, void *data, int flags) } if (res > 0) { /* Handled by this handler. Move to the next stream */ - session_media->handler = handler; + session_media_set_handler(session_media, handler); return CMP_MATCH; } } diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c index d1ec07610f..5b8d796dcb 100644 --- a/res/res_pjsip_t38.c +++ b/res/res_pjsip_t38.c @@ -816,6 +816,7 @@ static void stream_destroy(struct ast_sip_session_media *session_media) if (session_media->udptl) { ast_udptl_destroy(session_media->udptl); } + session_media->udptl = NULL; } /*! \brief SDP handler for 'image' media stream */