]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
AMI: Escape string values. 60/560/3
authorKevin Harwell <kharwell@digium.com>
Mon, 1 Jun 2015 16:45:30 +0000 (11:45 -0500)
committerKevin Harwell <kharwell@digium.com>
Wed, 3 Jun 2015 19:03:18 +0000 (14:03 -0500)
So this issue is a bit complicated. Since it is possible to pass values to AMI
that contain a '\r\n' (or other similar sequences) these values need to be
escaped. One way to solve this is to escape the values and then pass the escaped
values to the AMI variable parameter string building function. However, this
puts the onus on the pre-build function to escape all string values. This
potentially requires a fair amount of changes along with a lot of string
allocations/freeing for all values.

Surely there is a way to push this complexity down a level into the string
building function itself? This of course is possible, but ends up requiring a
way to distinguish between strings that need to be escaped and those that don't.
The best way to handle this is by introducing a new format specifier in the
format string. For instance a %s (no escape) and %S (escape). However, that is
a bit weird and unexpected.

So faced with those possibilities this patch implements a limited version of the
first option. Instead of attempting to escape all string values this patch only
escapes those values that make sense. This approach limits the number of changes
and doesn't suffer from the odd format specifier problem.

ASTERISK-24934 #close
Reported by: warren smith

Change-Id: Ib55a5b84fe0481b0f2caaaab68c566f392c0aac0

include/asterisk/strings.h
main/manager_channels.c
main/presencestate.c
main/stasis_channels.c
main/utils.c
tests/test_strings.c

index 7af92affc8b375232e31b90288fff54f7a010cca..d361293d0714125d1a364dce2ac1e3c0336e4265 100644 (file)
@@ -309,6 +309,59 @@ char *ast_unescape_semicolon(char *s);
  */
 char *ast_unescape_c(char *s);
 
+/*!
+ * \brief Escape the 'to_escape' characters in the given string.
+ *
+ * \note The given output buffer has to have enough memory allocated to store the
+ *       original string plus any escaped values.
+ *
+ * \param dest the escaped string
+ * \param s the source string to escape
+ * \param num number of characters to be copied from the source
+ * \param to_escape an array of characters to escape
+ *
+ * \return Pointer to the destination.
+ */
+char* ast_escape(char *dest, const char *s, size_t num, const char *to_escape);
+
+/*!
+ * \brief Escape standard 'C' sequences in the given string.
+ *
+ * \note The given output buffer has to have enough memory allocated to store the
+ *       original string plus any escaped values.
+ *
+ * \param dest the escaped string
+ * \param s the source string to escape
+ * \param num number of characters to be copied from the source
+ * \param to_escape an array of characters to escape
+ *
+ * \return Pointer to the escaped string.
+ */
+char* ast_escape_c(char *dest, const char *s, size_t num);
+
+/*!
+ * \brief Escape the 'to_escape' characters in the given string.
+ *
+ * \note Caller is responsible for freeing the returned string
+ *
+ * \param s the source string to escape
+ * \param to_escape an array of characters to escape
+ *
+ * \return Pointer to the escaped string or NULL.
+ */
+char *ast_escape_alloc(const char *s, const char *to_escape);
+
+/*!
+ * \brief Escape standard 'C' sequences in the given string.
+ *
+ * \note Caller is responsible for freeing the returned string
+ *
+ * \param s the source string to escape
+ *
+ * \return Pointer to the escaped string or NULL.
+ */
+char *ast_escape_c_alloc(const char *s);
+
 /*!
   \brief Size-limited null-terminating string copy.
   \param dst The destination buffer.
index 3aaab92d058506b13a068ba9807d5ee5b2de2506..552adc763e86f257eb5859b8a5d1dd21a698ecf3 100644 (file)
@@ -408,6 +408,7 @@ struct ast_str *ast_manager_build_channel_state_string_prefix(
 {
        struct ast_str *out = ast_str_create(1024);
        int res = 0;
+       char *caller_name, *connected_name;
 
        if (!out) {
                return NULL;
@@ -418,6 +419,9 @@ struct ast_str *ast_manager_build_channel_state_string_prefix(
                return NULL;
        }
 
+       caller_name = ast_escape_c_alloc(snapshot->caller_name);
+       connected_name = ast_escape_c_alloc(snapshot->connected_name);
+
        res = ast_str_set(&out, 0,
                "%sChannel: %s\r\n"
                "%sChannelState: %u\r\n"
@@ -436,9 +440,9 @@ struct ast_str *ast_manager_build_channel_state_string_prefix(
                prefix, snapshot->state,
                prefix, ast_state2str(snapshot->state),
                prefix, S_OR(snapshot->caller_number, "<unknown>"),
-               prefix, S_OR(snapshot->caller_name, "<unknown>"),
+               prefix, S_OR(caller_name, "<unknown>"),
                prefix, S_OR(snapshot->connected_number, "<unknown>"),
-               prefix, S_OR(snapshot->connected_name, "<unknown>"),
+               prefix, S_OR(connected_name, "<unknown>"),
                prefix, snapshot->language,
                prefix, snapshot->accountcode,
                prefix, snapshot->context,
@@ -448,18 +452,26 @@ struct ast_str *ast_manager_build_channel_state_string_prefix(
 
        if (!res) {
                ast_free(out);
+               ast_free(caller_name);
+               ast_free(connected_name);
                return NULL;
        }
 
        if (snapshot->manager_vars) {
                struct ast_var_t *var;
+               char *val;
                AST_LIST_TRAVERSE(snapshot->manager_vars, var, entries) {
+                       val = ast_escape_c_alloc(var->value);
                        ast_str_append(&out, 0, "%sChanVariable: %s=%s\r\n",
                                       prefix,
-                                      var->name, var->value);
+                                      var->name, S_OR(val, ""));
+                       ast_free(val);
                }
        }
 
+       ast_free(caller_name);
+       ast_free(connected_name);
+
        return out;
 }
 
@@ -556,6 +568,9 @@ static struct ast_manager_event_blob *channel_new_callerid(
        struct ast_channel_snapshot *old_snapshot,
        struct ast_channel_snapshot *new_snapshot)
 {
+       struct ast_manager_event_blob *res;
+       char *callerid;
+
        /* No NewCallerid event on cache clear or first event */
        if (!old_snapshot || !new_snapshot) {
                return NULL;
@@ -565,11 +580,19 @@ static struct ast_manager_event_blob *channel_new_callerid(
                return NULL;
        }
 
-       return ast_manager_event_blob_create(
+       if (!(callerid = ast_escape_c_alloc(
+                     ast_describe_caller_presentation(new_snapshot->caller_pres)))) {
+               return NULL;
+       }
+
+       res = ast_manager_event_blob_create(
                EVENT_FLAG_CALL, "NewCallerid",
                "CID-CallingPres: %d (%s)\r\n",
                new_snapshot->caller_pres,
-               ast_describe_caller_presentation(new_snapshot->caller_pres));
+               callerid);
+
+       ast_free(callerid);
+       return res;
 }
 
 static struct ast_manager_event_blob *channel_new_connected_line(
index ab03336e289af98a639c3886d0ddccd0365516e9..207e2aacb888da6384286f66edb5f570efcef095 100644 (file)
@@ -393,14 +393,23 @@ int ast_presence_state_engine_init(void)
 static struct ast_manager_event_blob *presence_state_to_ami(struct stasis_message *msg)
 {
        struct ast_presence_state_message *presence_state = stasis_message_data(msg);
+       struct ast_manager_event_blob *res;
 
-       return ast_manager_event_blob_create(EVENT_FLAG_CALL, "PresenceStateChange",
+       char *subtype = ast_escape_c_alloc(presence_state->subtype);
+       char *message = ast_escape_c_alloc(presence_state->message);
+
+       res = ast_manager_event_blob_create(EVENT_FLAG_CALL, "PresenceStateChange",
                "Presentity: %s\r\n"
                "Status: %s\r\n"
                "Subtype: %s\r\n"
                "Message: %s\r\n",
                presence_state->provider,
                ast_presence_state2str(presence_state->state),
-               presence_state->subtype,
-               presence_state->message);
+               subtype ?: "",
+                message ?: "");
+
+       ast_free(subtype);
+       ast_free(message);
+
+       return res;
 }
index ae76c1e846b2c7d04e78e378f04c3fd5ee5106f3..b8efd407b578bcb8507f0fca699bc7b524afe9fb 100644 (file)
@@ -793,8 +793,12 @@ static struct ast_manager_event_blob *varset_to_ami(struct stasis_message *msg)
        struct ast_channel_blob *obj = stasis_message_data(msg);
        const char *variable =
                ast_json_string_get(ast_json_object_get(obj->blob, "variable"));
-       const char *value =
-               ast_json_string_get(ast_json_object_get(obj->blob, "value"));
+       RAII_VAR(char *, value, ast_escape_c_alloc(
+                        ast_json_string_get(ast_json_object_get(obj->blob, "value"))), ast_free);
+
+       if (!value) {
+               return NULL;
+       }
 
        if (obj->snapshot) {
                channel_event_string =
@@ -1713,4 +1717,4 @@ static void remove_dial_masquerade_caller(struct ast_channel *caller)
 
        ast_channel_datastore_remove(caller, datastore);
        ast_datastore_free(datastore);
-}
\ No newline at end of file
+}
index d162cccf893e12c0142dea03678072790fe549a9..67c391ce246bc596077a5c3f938ab735325604a5 100644 (file)
@@ -1623,6 +1623,114 @@ char *ast_unescape_c(char *src)
        return ret;
 }
 
+/*
+ * Standard escape sequences - Note, '\0' is not included as a valid character
+ * to escape, but instead is used here as a NULL terminator for the string.
+ */
+char escape_sequences[] = {
+       '\a', '\b', '\f', '\n', '\r', '\t', '\v', '\\', '\'', '\"', '\?', '\0'
+};
+
+/*
+ * Standard escape sequences output map (has to maintain matching order with
+ * escape_sequences). '\0' is included here as a NULL terminator for the string.
+ */
+static char escape_sequences_map[] = {
+       'a', 'b', 'f', 'n', 'r', 't', 'v', '\\', '\'', '"', '?', '\0'
+};
+
+char* ast_escape(char *dest, const char *s, size_t num, const char *to_escape)
+{
+       char *p;
+
+       if (!dest || ast_strlen_zero(s)) {
+               return dest;
+       }
+
+       if (ast_strlen_zero(to_escape)) {
+               ast_copy_string(dest, s, num);
+               return dest;
+       }
+
+       for (p = dest; *s && num--; ++s, ++p) {
+               /* If in the list of characters to escape then escape it */
+               if (strchr(to_escape, *s)) {
+                       /*
+                        * See if the character to escape is part of the standard escape
+                        * sequences. If so we'll have to use its mapped counterpart
+                        * otherwise just use the current character.
+                        */
+                       char *c = strchr(escape_sequences, *s);
+                       *p++ = '\\';
+                       *p = c ? escape_sequences_map[c - escape_sequences] : *s;
+               } else {
+                       *p = *s;
+               }
+       }
+
+       *p = '\0';
+       return dest;
+}
+
+char* ast_escape_c(char *dest, const char *s, size_t num)
+{
+       /*
+        * Note - This is an optimized version of ast_escape. When looking only
+        * for escape_sequences a couple of checks used in the generic case can
+        * be left out thus making it slightly more efficient.
+        */
+       char *p;
+
+       if (!dest || ast_strlen_zero(s)) {
+               return dest;
+       }
+
+       for (p = dest; *s && num--; ++s, ++p) {
+               /*
+                * See if the character to escape is part of the standard escape
+                * sequences. If so use its mapped counterpart.
+                */
+               char *c = strchr(escape_sequences, *s);
+               if (c) {
+                       *p++ = '\\';
+                       *p = escape_sequences_map[c - escape_sequences];
+               } else {
+                       *p = *s;
+               }
+       }
+
+       *p = '\0';
+       return dest;
+}
+
+static char *escape_alloc(const char *s, size_t *size)
+{
+       if (!s || !(*size = strlen(s))) {
+               return NULL;
+       }
+
+       /*
+        * The result string needs to be twice the size of the given
+        * string just in case every character in it needs to be escaped.
+        */
+       *size = *size * 2 + 1;
+       return ast_calloc(sizeof(char), *size);
+}
+
+char *ast_escape_alloc(const char *s, const char *to_escape)
+{
+       size_t size = 0;
+       char *dest = escape_alloc(s, &size);
+       return ast_escape(dest, s, size, to_escape);
+}
+
+char *ast_escape_c_alloc(const char *s)
+{
+       size_t size = 0;
+       char *dest = escape_alloc(s, &size);
+       return ast_escape_c(dest, s, size);
+}
+
 int ast_build_string_va(char **buffer, size_t *space, const char *fmt, va_list ap)
 {
        int result;
index 4321d4a0392541a792ffd237c85af85c122682a7..ab65037eea575f9047b9be8b5f128d7485aff953 100644 (file)
@@ -455,6 +455,38 @@ AST_TEST_DEFINE(escape_semicolons_test)
        return AST_TEST_PASS;
 }
 
+AST_TEST_DEFINE(escape_test)
+{
+       char buf[128];
+
+#define TEST_ESCAPE(s, to_escape, expected) \
+       !strcmp(ast_escape(buf, s, sizeof(buf) / sizeof(char), to_escape), expected)
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "escape";
+               info->category = "/main/strings/";
+               info->summary = "Test ast_escape";
+               info->description = "Test escaping values in a string";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       ast_test_validate(test, TEST_ESCAPE("null escape", NULL, "null escape"));
+       ast_test_validate(test, TEST_ESCAPE("no matching escape", "Z", "no matching escape"));
+       ast_test_validate(test, TEST_ESCAPE("escape Z", "Z", "escape \\Z"));
+       ast_test_validate(test, TEST_ESCAPE("Z", "Z", "\\Z"));
+       ast_test_validate(test, TEST_ESCAPE(";;", ";;", "\\;\\;"));
+       ast_test_validate(test, TEST_ESCAPE("escape \n", "\n", "escape \\n"));
+       ast_test_validate(test, TEST_ESCAPE("escape \n again \n", "\n", "escape \\n again \\n"));
+
+       ast_test_validate(test, !strcmp(ast_escape_c(buf, "escape \a\b\f\n\r\t\v\\\'\"\?",
+                                                    sizeof(buf) / sizeof(char)),
+                                       "escape \\a\\b\\f\\n\\r\\t\\v\\\\\\\'\\\"\\?"));
+       return AST_TEST_PASS;
+}
+
 static int unload_module(void)
 {
        AST_TEST_UNREGISTER(str_test);
@@ -462,6 +494,7 @@ static int unload_module(void)
        AST_TEST_UNREGISTER(ends_with_test);
        AST_TEST_UNREGISTER(strsep_test);
        AST_TEST_UNREGISTER(escape_semicolons_test);
+       AST_TEST_UNREGISTER(escape_test);
        return 0;
 }
 
@@ -472,6 +505,7 @@ static int load_module(void)
        AST_TEST_REGISTER(ends_with_test);
        AST_TEST_REGISTER(strsep_test);
        AST_TEST_REGISTER(escape_semicolons_test);
+       AST_TEST_REGISTER(escape_test);
        return AST_MODULE_LOAD_SUCCESS;
 }