[asterisk-commits] file: trunk r429274 - in /trunk: ./ channels/ res/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Dec 10 07:35:56 CST 2014


Author: file
Date: Wed Dec 10 07:35:52 2014
New Revision: 429274

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=429274
Log:
res_http_websocket: Fix crash due to double freeing memory when receiving a payload length of zero.

Frames with a payload length of 0 were incorrectly handled in res_http_websocket.
Provided a frame with a payload had been received prior it was possible for a double
free to occur. The realloc operation would succeed (thus freeing the payload) but be
treated as an error. When the session was then torn down the payload would be
freed again causing a crash. The read function now takes this into account.

This change also fixes assumptions made by users of res_http_websocket. There is no
guarantee that a frame received from it will be NULL terminated.

ASTERISK-24472 #close
Reported by: Badalian Vyacheslav

Review: https://reviewboard.asterisk.org/r/4220/
Review: https://reviewboard.asterisk.org/r/4219/
........

Merged revisions 429270 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 429272 from http://svn.asterisk.org/svn/asterisk/branches/12
........

Merged revisions 429273 from http://svn.asterisk.org/svn/asterisk/branches/13

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c
    trunk/res/res_http_websocket.c
    trunk/res/res_pjsip_transport_websocket.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-13-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=429274&r1=429273&r2=429274
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Wed Dec 10 07:35:52 2014
@@ -2648,12 +2648,16 @@
 
 		if (opcode == AST_WEBSOCKET_OPCODE_TEXT || opcode == AST_WEBSOCKET_OPCODE_BINARY) {
 			struct sip_request req = { 0, };
+			char data[payload_len + 1];
 
 			if (!(req.data = ast_str_create(payload_len + 1))) {
 				goto end;
 			}
 
-			if (ast_str_set(&req.data, -1, "%s", payload) == AST_DYNSTR_BUILD_FAILED) {
+			strncpy(data, payload, payload_len);
+			data[payload_len] = '\0';
+
+			if (ast_str_set(&req.data, -1, "%s", data) == AST_DYNSTR_BUILD_FAILED) {
 				deinit_req(&req);
 				goto end;
 			}

Modified: trunk/res/res_http_websocket.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_http_websocket.c?view=diff&rev=429274&r1=429273&r2=429274
==============================================================================
--- trunk/res/res_http_websocket.c (original)
+++ trunk/res/res_http_websocket.c Wed Dec 10 07:35:52 2014
@@ -513,14 +513,6 @@
 			}
 		}
 
-		if (!(new_payload = ast_realloc(session->payload, (session->payload_len + *payload_len)))) {
-			ast_log(LOG_WARNING, "Failed allocation: %p, %zu, %"PRIu64"\n",
-				session->payload, session->payload_len, *payload_len);
-			*payload_len = 0;
-			ast_websocket_close(session, 1009);
-			return 0;
-		}
-
 		/* Per the RFC for PING we need to send back an opcode with the application data as received */
 		if ((*opcode == AST_WEBSOCKET_OPCODE_PING) && (ast_websocket_write(session, AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len))) {
 			*payload_len = 0;
@@ -528,9 +520,22 @@
 			return 0;
 		}
 
-		session->payload = new_payload;
-		memcpy((session->payload + session->payload_len), (*payload), (*payload_len));
-		session->payload_len += *payload_len;
+		if (*payload_len) {
+			if (!(new_payload = ast_realloc(session->payload, (session->payload_len + *payload_len)))) {
+				ast_log(LOG_WARNING, "Failed allocation: %p, %zu, %"PRIu64"\n",
+					session->payload, session->payload_len, *payload_len);
+				*payload_len = 0;
+				ast_websocket_close(session, 1009);
+				return 0;
+			}
+
+			session->payload = new_payload;
+			memcpy((session->payload + session->payload_len), (*payload), (*payload_len));
+			session->payload_len += *payload_len;
+		} else if (!session->payload_len && session->payload) {
+			ast_free(session->payload);
+			session->payload = NULL;
+		}
 
 		if (!fin && session->reconstruct && (session->payload_len < session->reconstruct)) {
 			/* If this is not a final message we need to defer returning it until later */

Modified: trunk/res/res_pjsip_transport_websocket.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_pjsip_transport_websocket.c?view=diff&rev=429274&r1=429273&r2=429274
==============================================================================
--- trunk/res/res_pjsip_transport_websocket.c (original)
+++ trunk/res/res_pjsip_transport_websocket.c Wed Dec 10 07:35:52 2014
@@ -200,7 +200,8 @@
 
 	pj_gettimeofday(&rdata->pkt_info.timestamp);
 
-	pj_memcpy(rdata->pkt_info.packet, read_data->payload, sizeof(rdata->pkt_info.packet));
+	pj_memcpy(rdata->pkt_info.packet, read_data->payload,
+		PJSIP_MAX_PKT_LEN < read_data->payload_len ? PJSIP_MAX_PKT_LEN : read_data->payload_len);
 	rdata->pkt_info.len = read_data->payload_len;
 	rdata->pkt_info.zero = 0;
 




More information about the asterisk-commits mailing list