diff --git a/ChangeLog b/ChangeLog index dbac521833..7424fe8f74 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2014-12-10 Asterisk Development Team + + * Asterisk 11.14.2 Released. + + * AST-2014-019: Fix crash when receiving a WebSocket packet with 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 + 2014-11-20 Asterisk Development Team * Asterisk 11.14.1 Released. diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 08f3c9ebf6..a9c9982ed4 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -2595,12 +2595,16 @@ static void sip_websocket_callback(struct ast_websocket *session, struct ast_var 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; } diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c index 5258a5f556..81fa83d1e8 100644 --- a/res/res_http_websocket.c +++ b/res/res_http_websocket.c @@ -462,14 +462,6 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, cha } } - 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; @@ -477,9 +469,22 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, cha 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 */