diff --git a/res/res_pjsip.c b/res/res_pjsip.c index 4de07459752c0c20399ff280e1562eb4d3163907..25a35c0c805951fb766dc6e427056f2fd5455431 100644 --- a/res/res_pjsip.c +++ b/res/res_pjsip.c @@ -40,6 +40,8 @@ #include "asterisk/file.h" #include "asterisk/cli.h" #include "asterisk/res_pjsip_cli.h" +#include "asterisk/test.h" +#include "asterisk/res_pjsip_presence_xml.h" /*** MODULEINFO <depend>pjproject</depend> @@ -3755,6 +3757,57 @@ static void remove_request_headers(pjsip_endpoint *endpt) } } +AST_TEST_DEFINE(xml_sanitization_end_null) +{ + char sanitized[8]; + + switch (cmd) { + case TEST_INIT: + info->name = "xml_sanitization_end_null"; + info->category = "/res/res_pjsip/"; + info->summary = "Ensure XML sanitization works as expected with a long string"; + info->description = "This test sanitizes a string which exceeds the output\n" + "buffer size. Once done the string is confirmed to be NULL terminated."; + return AST_TEST_NOT_RUN; + case TEST_EXECUTE: + break; + } + + ast_sip_sanitize_xml("aaaaaaaaaaaa", sanitized, sizeof(sanitized)); + if (sanitized[7] != '\0') { + ast_test_status_update(test, "Sanitized XML string is not null-terminated when it should be\n"); + return AST_TEST_FAIL; + } + + return AST_TEST_PASS; +} + +AST_TEST_DEFINE(xml_sanitization_exceeds_buffer) +{ + char sanitized[8]; + + switch (cmd) { + case TEST_INIT: + info->name = "xml_sanitization_exceeds_buffer"; + info->category = "/res/res_pjsip/"; + info->summary = "Ensure XML sanitization does not exceed buffer when output won't fit"; + info->description = "This test sanitizes a string which before sanitization would\n" + "fit within the output buffer. After sanitization, however, the string would\n" + "exceed the buffer. Once done the string is confirmed to be NULL terminated."; + return AST_TEST_NOT_RUN; + case TEST_EXECUTE: + break; + } + + ast_sip_sanitize_xml("<><><>&", sanitized, sizeof(sanitized)); + if (sanitized[7] != '\0') { + ast_test_status_update(test, "Sanitized XML string is not null-terminated when it should be\n"); + return AST_TEST_FAIL; + } + + return AST_TEST_PASS; +} + /*! * \internal * \brief Reload configuration within a PJSIP thread @@ -3909,6 +3962,9 @@ static int load_module(void) ast_res_pjsip_init_options_handling(0); ast_cli_register_multiple(cli_commands, ARRAY_LEN(cli_commands)); + AST_TEST_REGISTER(xml_sanitization_end_null); + AST_TEST_REGISTER(xml_sanitization_exceeds_buffer); + return AST_MODULE_LOAD_SUCCESS; } @@ -3950,6 +4006,9 @@ static int unload_pjsip(void *data) static int unload_module(void) { + AST_TEST_UNREGISTER(xml_sanitization_end_null); + AST_TEST_UNREGISTER(xml_sanitization_exceeds_buffer); + /* The thread this is called from cannot call PJSIP/PJLIB functions, * so we have to push the work to the threadpool to handle */ diff --git a/res/res_pjsip/presence_xml.c b/res/res_pjsip/presence_xml.c index 12bfa078c5e050d38947cc0e8429029dacc10253..b98ea0237e960e537ed16bb1daad3c5340ac05cc 100644 --- a/res/res_pjsip/presence_xml.c +++ b/res/res_pjsip/presence_xml.c @@ -30,45 +30,54 @@ void ast_sip_sanitize_xml(const char *input, char *output, size_t len) { char *copy = ast_strdupa(input); char *break_point; + size_t remaining = len - 1; output[0] = '\0'; - while ((break_point = strpbrk(copy, "<>\"&'\n\r"))) { + while ((break_point = strpbrk(copy, "<>\"&'\n\r")) && remaining) { char to_escape = *break_point; *break_point = '\0'; - strncat(output, copy, len); + strncat(output, copy, remaining); + + /* The strncat function will write remaining+1 if the string length is + * equal to or greater than the size provided to it. We take this into + * account by subtracting 1, which ensures that the NULL byte is written + * inside of the provided buffer. + */ + remaining = len - strlen(output) - 1; switch (to_escape) { case '<': - strncat(output, "<", len); + strncat(output, "<", remaining); break; case '>': - strncat(output, ">", len); + strncat(output, ">", remaining); break; case '"': - strncat(output, """, len); + strncat(output, """, remaining); break; case '&': - strncat(output, "&", len); + strncat(output, "&", remaining); break; case '\'': - strncat(output, "'", len); + strncat(output, "'", remaining); break; case '\r': - strncat(output, " ", len); + strncat(output, " ", remaining); break; case '\n': - strncat(output, " ", len); + strncat(output, " ", remaining); break; }; copy = break_point + 1; + remaining = len - strlen(output) - 1; } /* Be sure to copy everything after the final bracket */ - if (*copy) { - strncat(output, copy, len); + if (*copy && remaining) { + strncat(output, copy, remaining); } } diff --git a/res/res_pjsip_pidf_digium_body_supplement.c b/res/res_pjsip_pidf_digium_body_supplement.c index b6212ea5e48f18260154170cc75db729784f92b1..93e4982e28e9e1491d990cdb5191a663043a5186 100644 --- a/res/res_pjsip_pidf_digium_body_supplement.c +++ b/res/res_pjsip_pidf_digium_body_supplement.c @@ -40,7 +40,7 @@ static int pidf_supplement_body(void *body, void *data) { struct ast_sip_exten_state_data *state_data = data; pj_xml_node *node; - char sanitized[256]; + char sanitized[1024]; if (ast_strlen_zero(state_data->user_agent) || !strstr(state_data->user_agent, "digium")) {