Merge r431301, 431303 for 13.1.1

git-svn-id: https://origsvn.digium.com/svn/asterisk/tags/13.1.1@431327 65c4cc65-6c06-0410-ace0-fbb531ad65f3

ChangeLog

@@ -1,3 +1,38 @@
+2015-01-28  Asterisk Development Team <asteriskteam@digium.com>
+
+	* 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 <asteriskteam@digium.com>
 
 	* Asterisk 13.1.0 Released.

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
 	<function name="CURL" language="en_US">
@@ -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;
 }
 

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 */

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;
 		}
 	}

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 */