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

diff --git a/include/asterisk/res_pjsip_presence_xml.h b/include/asterisk/res_pjsip_presence_xml.h
index add5f89189a2ca385e53f6e61f56feb0a2fa05ee..deed0901e49ebcb02508d3fe0c109f61dda1e0a4 100644 (file)
--- 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 d9725f4c53c5187d869fd9b017cd01fec9e29887..48ac60f98a1d4472fc4d223e946f824d84bdefe6 100644 (file)
--- 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 ef0cce599aa05ce1afa410f47195b6854498c313..d3be8c131f7589af86159c7848459847b49053a0 100644 (file)
--- 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 c00bc76ee6cd18c2580fc19554b25a63817da878..650f5c5c8afb5a00d37a9cc4cf36dcef4155ca99 100644 (file)
--- 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 43cb1e78bc3371c5e19335d74c4593306eb48cab..298235cbce481605f997ad7fbe95e2b2ff24349e 100644 (file)
--- 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;
        }