summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRichard Mudgett <rmudgett@digium.com>2015-07-02 14:51:29 -0500
committerRichard Mudgett <rmudgett@digium.com>2015-07-06 16:15:12 -0500
commit7cd99be534945fffd5927554c9c4469454008d3c (patch)
treedd067111bae6b7adae9d8cbddb48b96664522600
parent792ed7ce93205fc700b3923640fa023b34eff5cb (diff)
PJSIP XML, XPIDF: Fix buffer size overwrite memory corruption error.
When res_pjsip body generator modules were generating XML or XPIDF response bodies, there was a chance that the generated body would be the exact size of the supplied buffer. Adding the nul string terminator would then write beyond the end of the buffer and potentially corrupt memory. * Fix MALLOC_DEBUG high fence violations caused by adding a nul string terminator on the end of a buffer for XML or XPIDF response bodies. * Made calls to pj_xml_print() safer if the XML prolog is requested. Due to a bug in pjproject, the return value could be -1 _or_ AST_PJSIP_XML_PROLOG_LEN if the supplied buffer is not large enough. * Updated the doxygen comment of AST_PJSIP_XML_PROLOG_LEN to describe the return value of pj_xml_print() when the supplied buffer is not large enough. ASTERISK-25168 Reported by: Carl Fortin Change-Id: Id70e1d373a6a2b2bd9e678b5cbc5e55b308981de
-rw-r--r--include/asterisk/res_pjsip_presence_xml.h9
-rw-r--r--res/res_pjsip_dialog_info_body_generator.c9
-rw-r--r--res/res_pjsip_pidf_body_generator.c11
-rw-r--r--res/res_pjsip_pubsub.c2
-rw-r--r--res/res_pjsip_xpidf_body_generator.c9
5 files changed, 19 insertions, 21 deletions
diff --git a/include/asterisk/res_pjsip_presence_xml.h b/include/asterisk/res_pjsip_presence_xml.h
index add5f8918..deed0901e 100644
--- a/include/asterisk/res_pjsip_presence_xml.h
+++ b/include/asterisk/res_pjsip_presence_xml.h
@@ -17,14 +17,15 @@
*/
/*!
- * \brief The length of the XML prolog when printing
- * presence or other XML in PJSIP.
+ * \brief Length of the XML prolog when printing presence or other XML in PJSIP.
*
* When calling any variant of pj_xml_print(), the documentation
* claims that it will return -1 if the provided buffer is not
* large enough. However, if the XML prolog is requested to be
- * printed, then the length of the XML prolog is returned upon
- * failure instead of -1.
+ * printed and the buffer is not large enough, then it will
+ * return -1 only if the buffer is not large enough to hold the
+ * XML prolog or return the length of the XML prolog on failure
+ * instead of -1.
*
* This constant is useful to check against when trying to determine
* if printing XML succeeded or failed.
diff --git a/res/res_pjsip_dialog_info_body_generator.c b/res/res_pjsip_dialog_info_body_generator.c
index c8dc4014e..e48057ba4 100644
--- a/res/res_pjsip_dialog_info_body_generator.c
+++ b/res/res_pjsip_dialog_info_body_generator.c
@@ -163,14 +163,13 @@ static void dialog_info_to_string(void *body, struct ast_str **str)
int size;
do {
- size = pj_xml_print(dialog_info, ast_str_buffer(*str), ast_str_size(*str), PJ_TRUE);
- if (size == AST_PJSIP_XML_PROLOG_LEN) {
+ size = pj_xml_print(dialog_info, ast_str_buffer(*str), ast_str_size(*str) - 1, PJ_TRUE);
+ if (size <= AST_PJSIP_XML_PROLOG_LEN) {
ast_str_make_space(str, ast_str_size(*str) * 2);
++growths;
}
- } while (size == AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
-
- if (size == AST_PJSIP_XML_PROLOG_LEN) {
+ } while (size <= AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
+ if (size <= AST_PJSIP_XML_PROLOG_LEN) {
ast_log(LOG_WARNING, "dialog-info+xml body text too large\n");
return;
}
diff --git a/res/res_pjsip_pidf_body_generator.c b/res/res_pjsip_pidf_body_generator.c
index 0863eafb3..25f063915 100644
--- a/res/res_pjsip_pidf_body_generator.c
+++ b/res/res_pjsip_pidf_body_generator.c
@@ -84,19 +84,18 @@ static int pidf_generate_body_content(void *body, void *data)
static void pidf_to_string(void *body, struct ast_str **str)
{
- int size;
- int growths = 0;
pjpidf_pres *pres = body;
+ int growths = 0;
+ int size;
do {
size = pjpidf_print(pres, ast_str_buffer(*str), ast_str_size(*str) - 1);
- if (size == AST_PJSIP_XML_PROLOG_LEN) {
+ if (size <= AST_PJSIP_XML_PROLOG_LEN) {
ast_str_make_space(str, ast_str_size(*str) * 2);
++growths;
}
- } while (size == AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
-
- if (size == AST_PJSIP_XML_PROLOG_LEN) {
+ } while (size <= AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
+ if (size <= AST_PJSIP_XML_PROLOG_LEN) {
ast_log(LOG_WARNING, "PIDF body text too large\n");
return;
}
diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index c01c7c473..76c9b3367 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -1769,7 +1769,7 @@ static int rlmi_print_body(struct pjsip_msg_body *msg_body, char *buf, pj_size_t
pj_xml_node *rlmi = msg_body->data;
num_printed = pj_xml_print(rlmi, buf, size, PJ_TRUE);
- if (num_printed == AST_PJSIP_XML_PROLOG_LEN) {
+ if (num_printed <= AST_PJSIP_XML_PROLOG_LEN) {
return -1;
}
diff --git a/res/res_pjsip_xpidf_body_generator.c b/res/res_pjsip_xpidf_body_generator.c
index 37676ff96..924046549 100644
--- a/res/res_pjsip_xpidf_body_generator.c
+++ b/res/res_pjsip_xpidf_body_generator.c
@@ -106,14 +106,13 @@ static void xpidf_to_string(void *body, struct ast_str **str)
int size;
do {
- size = pjxpidf_print(pres, ast_str_buffer(*str), ast_str_size(*str));
- if (size == AST_PJSIP_XML_PROLOG_LEN) {
+ size = pjxpidf_print(pres, ast_str_buffer(*str), ast_str_size(*str) - 1);
+ if (size <= AST_PJSIP_XML_PROLOG_LEN) {
ast_str_make_space(str, ast_str_size(*str) * 2);
++growths;
}
- } while (size == AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
-
- if (size == AST_PJSIP_XML_PROLOG_LEN) {
+ } while (size <= AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
+ if (size <= AST_PJSIP_XML_PROLOG_LEN) {
ast_log(LOG_WARNING, "XPIDF body text too large\n");
return;
}