[Asterisk-code-review] AST-2016-004: Fix crash on REGISTER with long URI. (asterisk[certified/13.8])
Mark Michelson
asteriskteam at digium.com
Thu Apr 14 11:29:16 CDT 2016
Mark Michelson has uploaded a new change for review.
https://gerrit.asterisk.org/2608
Change subject: AST-2016-004: Fix crash on REGISTER with long URI.
......................................................................
AST-2016-004: Fix crash on REGISTER with long URI.
Due to some ignored return values, Asterisk could crash if processing an
incoming REGISTER whose contact URI was above a certain length.
ASTERISK-25707 #close
Reported by George Joseph
Patches:
0001-res_pjsip-Validate-that-URIs-don-t-exceed-pjproject-.patch
AST-2016-004
Change-Id: I0ed3898fe7ab10121b76c8c79046692de3a1be55
---
M res/res_pjsip/include/res_pjsip_private.h
M res/res_pjsip/location.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_registrar.c
4 files changed, 93 insertions(+), 2 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/2608/1
diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h
index c1f7e23..72a4387 100644
--- a/res/res_pjsip/include/res_pjsip_private.h
+++ b/res/res_pjsip/include/res_pjsip_private.h
@@ -318,4 +318,11 @@
* \brief Finds or creates contact_status for a contact
*/
struct ast_sip_contact_status *ast_res_pjsip_find_or_create_contact_status(const struct ast_sip_contact *contact);
+
+/*!
+ * \internal
+ * \brief Validate that the uri meets pjproject length restrictions
+ */
+int ast_sip_validate_uri_length(const char *uri);
+
#endif /* RES_PJSIP_PRIVATE_H_ */
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 4284f35..f3398e0 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -28,6 +28,11 @@
#include "asterisk/res_pjsip_cli.h"
#include "asterisk/statsd.h"
+#include "asterisk/res_pjproject.h"
+
+static int pj_max_hostname = PJ_MAX_HOSTNAME;
+static int pjsip_max_url_size = PJSIP_MAX_URL_SIZE;
+
/*! \brief Destructor for AOR */
static void aor_destroy(void *obj)
{
@@ -369,6 +374,43 @@
return cmp;
}
+int ast_sip_validate_uri_length(const char *contact_uri)
+{
+ pjsip_uri *uri;
+ pjsip_sip_uri *sip_uri;
+ pj_pool_t *pool;
+ int max_length = pj_max_hostname - 1;
+
+ if (strlen(contact_uri) > pjsip_max_url_size - 1) {
+ return -1;
+ }
+
+ if (!(pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "uri validation", 512, 512))) {
+ ast_log(LOG_ERROR, "Unable to allocate pool for uri validation\n");
+ return -1;
+ }
+
+ if (!(uri = pjsip_parse_uri(pool, (char *)contact_uri, strlen(contact_uri), 0)) ||
+ (!PJSIP_URI_SCHEME_IS_SIP(uri) && !PJSIP_URI_SCHEME_IS_SIPS(uri))) {
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+ return -1;
+ }
+
+ sip_uri = pjsip_uri_get_uri(uri);
+ if (sip_uri->port == 0) {
+ max_length -= strlen("_sips.tcp.");
+ }
+
+ if (sip_uri->host.slen > max_length) {
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+ return -1;
+ }
+
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+
+ return 0;
+}
+
/*! \brief Custom handler for permanent URIs */
static int permanent_uri_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
{
@@ -390,6 +432,11 @@
if (ast_strlen_zero(contact_uri)) {
continue;
+ }
+
+ if (ast_sip_validate_uri_length(contact_uri)) {
+ ast_log(LOG_ERROR, "Contact uri or hostname length exceeds pjproject limit: %s\n", contact_uri);
+ return -1;
}
if (!aor->permanent_contacts) {
@@ -961,6 +1008,10 @@
struct ast_sorcery *sorcery = ast_sip_get_sorcery();
int i;
+ ast_pjproject_get_buildopt("PJ_MAX_HOSTNAME", "%d", &pj_max_hostname);
+ /* As of pjproject 2.4.5, PJSIP_MAX_URL_SIZE isn't exposed yet but we try anyway. */
+ ast_pjproject_get_buildopt("PJSIP_MAX_URL_SIZE", "%d", &pjsip_max_url_size);
+
ast_sorcery_apply_default(sorcery, "contact", "astdb", "registrar");
ast_sorcery_apply_default(sorcery, "aor", "config", "pjsip.conf,criteria=type=aor");
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 59c3db5..8a40cce 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -1290,10 +1290,18 @@
ast_log(LOG_ERROR, "No server URI specified on outbound registration '%s'\n",
ast_sorcery_object_get_id(applied));
return -1;
+ } else if (ast_sip_validate_uri_length(applied->server_uri)) {
+ ast_log(LOG_ERROR, "Server URI or hostname length exceeds pjpropject limit '%s'\n",
+ ast_sorcery_object_get_id(applied));
+ return -1;
} else if (ast_strlen_zero(applied->client_uri)) {
ast_log(LOG_ERROR, "No client URI specified on outbound registration '%s'\n",
ast_sorcery_object_get_id(applied));
return -1;
+ } else if (ast_sip_validate_uri_length(applied->client_uri)) {
+ ast_log(LOG_ERROR, "Client URI or hostname length exceeds pjpropject limit '%s'\n",
+ ast_sorcery_object_get_id(applied));
+ return -1;
} else if (applied->line && ast_strlen_zero(applied->endpoint)) {
ast_log(LOG_ERROR, "Line support has been enabled on outbound registration '%s' without providing an endpoint\n",
ast_sorcery_object_get_id(applied));
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index fb2b9da..b035d52 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -18,6 +18,7 @@
/*** MODULEINFO
<depend>pjproject</depend>
+ <depend>res_pjproject</depend>
<depend>res_pjsip</depend>
<support_level>core</support_level>
***/
@@ -32,6 +33,7 @@
#include "asterisk/test.h"
#include "asterisk/taskprocessor.h"
#include "asterisk/manager.h"
+#include "asterisk/res_pjproject.h"
#include "res_pjsip/include/res_pjsip_private.h"
/*** DOCUMENTATION
@@ -50,6 +52,9 @@
</description>
</manager>
***/
+
+static int pj_max_hostname = PJ_MAX_HOSTNAME;
+static int pjsip_max_url_size = PJSIP_MAX_URL_SIZE;
/*! \brief Internal function which returns the expiration time for a contact */
static int registrar_get_expiration(const struct ast_sip_aor *aor, const pjsip_contact_hdr *contact, const pjsip_rx_data *rdata)
@@ -85,7 +90,7 @@
/*! \brief Pool used for parsing URI */
pj_pool_t *pool;
/*! \brief URI being looked for */
- pjsip_uri *uri;
+ pjsip_sip_uri *uri;
};
/*! \brief Callback function for finding a contact */
@@ -113,6 +118,7 @@
while ((contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next))) {
int expiration = registrar_get_expiration(aor, contact, rdata);
RAII_VAR(struct ast_sip_contact *, existing, NULL, ao2_cleanup);
+ char contact_uri[pjsip_max_url_size];
if (contact->star) {
/* The expiration MUST be 0 when a '*' contact is used and there must be no other contact */
@@ -133,6 +139,19 @@
}
details.uri = pjsip_uri_get_uri(contact->uri);
+
+ /* pjsip_uri_print returns -1 if there's not enough room in the buffer */
+ if (pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, details.uri, contact_uri, sizeof(contact_uri)) < 0) {
+ /* If the total length of the uri is greater than pjproject can handle, go no further */
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+ return -1;
+ }
+
+ if (details.uri->host.slen >= pj_max_hostname) {
+ /* If the length of the hostname is greater than pjproject can handle, go no further */
+ pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+ return -1;
+ }
/* Determine if this is an add, update, or delete for policy enforcement purposes */
if (!(existing = ao2_callback(contacts, 0, registrar_find_contact, &details))) {
@@ -477,7 +496,7 @@
/* Iterate each provided Contact header and add, update, or delete */
while ((contact_hdr = pjsip_msg_find_hdr(task_data->rdata->msg_info.msg, PJSIP_H_CONTACT, contact_hdr ? contact_hdr->next : NULL))) {
int expiration;
- char contact_uri[PJSIP_MAX_URL_SIZE];
+ char contact_uri[pjsip_max_url_size];
RAII_VAR(struct ast_sip_contact *, contact, NULL, ao2_cleanup);
if (contact_hdr->star) {
@@ -800,6 +819,12 @@
{
const pj_str_t STR_REGISTER = { "REGISTER", 8 };
+ CHECK_PJPROJECT_MODULE_LOADED();
+
+ ast_pjproject_get_buildopt("PJ_MAX_HOSTNAME", "%d", &pj_max_hostname);
+ /* As of pjproject 2.4.5, PJSIP_MAX_URL_SIZE isn't exposed yet but we try anyway. */
+ ast_pjproject_get_buildopt("PJSIP_MAX_URL_SIZE", "%d", &pjsip_max_url_size);
+
CHECK_PJSIP_MODULE_LOADED();
if (!(serializers = ao2_container_alloc(
--
To view, visit https://gerrit.asterisk.org/2608
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0ed3898fe7ab10121b76c8c79046692de3a1be55
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.8
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list