Lookups: avoid leaking user/passwd from server spec to log. Bug 3066
[exim.git] / src / src / search.c
index 421d9c789c0ef08d06c29079bf39c61990c533c8..62416da76903d77a7dcddc188cdceada237f873e 100644 (file)
@@ -2,9 +2,10 @@
 *     Exim - an Internet mail transport agent    *
 *************************************************/
 
+/* Copyright (c) The Exim Maintainers 2020 - 2023 */
 /* Copyright (c) University of Cambridge 1995 - 2015 */
-/* Copyright (c) The Exim Maintainers 2020 */
 /* See the file NOTICE for conditions of use and distribution. */
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 
 /* A set of functions to search databases in various formats. An open
 database is represented by a void * value which is returned from a lookup-
@@ -63,11 +64,9 @@ Returns:     +ve => valid lookup name; value is offset in lookup_list
 */
 
 int
-search_findtype(const uschar *name, int len)
+search_findtype(const uschar * name, int len)
 {
-int bot = 0;
-int top = lookup_list_count;
-while (top > bot)
+for (int bot = 0, top = lookup_list_count; top > bot; )
   {
   int mid = (top + bot)/2;
   int c = Ustrncmp(name, lookup_list[mid]->name, len);
@@ -92,7 +91,7 @@ while (top > bot)
   if (c > 0) bot = mid + 1; else top = mid;
   }
 
-search_error_message = string_sprintf("unknown lookup type \"%.*s\"",len,name);
+search_error_message = string_sprintf("unknown lookup type \"%.*s\"", len, name);
 return -1;
 }
 
@@ -478,8 +477,8 @@ count alone. */
 
 if (!t)
   {
-  t = store_get(sizeof(tree_node) + Ustrlen(keybuffer), FALSE);
-  t->data.ptr = c = store_get(sizeof(search_cache), FALSE);
+  t = store_get(sizeof(tree_node) + Ustrlen(keybuffer), GET_UNTAINTED);
+  t->data.ptr = c = store_get(sizeof(search_cache), GET_UNTAINTED);
   c->item_cache = NULL;
   Ustrcpy(t->name, keybuffer);
   tree_insertnode(&search_tree, t);
@@ -580,6 +579,60 @@ else
       filename ? US"file" : US"database",
       keystring,
       filename ? US"\n  in " : US"", filename ? filename : US"");
+    if (!filename && is_tainted(keystring))
+      {
+      debug_printf_indent("                             ");
+      debug_print_taint(keystring);
+      }
+    }
+
+  /* Check that the query, for query-style lookups,
+  is either untainted or properly quoted for the lookup type.
+
+  XXX Should we this move into lf_sqlperform() ?  The server-taint check is there.
+  Also it already knows about looking for a "servers" spec in the query string.
+  Passing search_type down that far is an issue.
+  */
+
+  if (  !filename && lookup_list[search_type]->quote
+     && is_tainted(keystring) && !is_quoted_like(keystring, search_type))
+    {
+    const uschar * ks = keystring;
+    uschar * loc = acl_current_verb();
+    if (!loc) loc = authenticator_current_name();      /* must be before transport */
+    if (!loc) loc = transport_current_name();          /* must be before router */
+    if (!loc) loc = router_current_name();             /* GCC ?: would be good, but not in clang */
+    if (!loc) loc = US"";
+
+    if (Ustrncmp(ks, "servers", 7) == 0)       /* Avoid logging server/password */
+      if ((ks = Ustrchr(keystring, ';')))
+       while (isspace(*++ks))
+         ;
+      else
+       ks = US"";
+
+#ifdef enforce_quote_protection_notyet
+    search_error_message = string_sprintf(
+      "tainted search query is not properly quoted%s: %s%s",
+      loc, ks);
+    f.search_find_defer = TRUE;
+    goto out;
+#else
+    /* If we're called from a transport, no privs to open the paniclog;
+    the logging punts to using stderr - and that seems to stop the debug
+    stream. */
+    log_write(0,
+      transport_name ? LOG_MAIN : LOG_MAIN|LOG_PANIC,
+      "tainted search query is not properly quoted%s: %s", loc, ks);
+
+    DEBUG(D_lookup)
+      {
+      int q = quoter_for_address(ks);
+      debug_printf_indent("search_type %d (%s) quoting %d (%s)\n",
+       search_type, lookup_list[search_type]->name,
+       q, is_real_quoter(q) ? lookup_list[q]->name : US"none");
+      }
+#endif
     }
 
   /* Call the code for the different kinds of search. DEFER is handled
@@ -587,7 +640,7 @@ else
   distinguish if necessary. */
 
   if (lookup_list[search_type]->find(c->handle, filename, keystring, keylength,
-      &data, &search_error_message, &do_cache, opts) == DEFER)
+         &data, &search_error_message, &do_cache, opts) == DEFER)
     f.search_find_defer = TRUE;
 
   /* A record that has been found is now in data, which is either NULL
@@ -605,8 +658,8 @@ else
     if (!t)    /* No existing entry.  Create new one. */
       {
       int len = keylength + 1;
-      e = store_get(sizeof(expiring_data) + sizeof(tree_node) + len,
-                   is_tainted(keystring));
+      /* The cache node value should never be expanded so use tainted mem */
+      e = store_get(sizeof(expiring_data) + sizeof(tree_node) + len, GET_TAINTED);
       t = (tree_node *)(e+1);
       memcpy(t->name, keystring, len);
       t->data.ptr = e;
@@ -619,8 +672,8 @@ else
     e->data.ptr = data;
     }
 
-  /* If caching was disabled, empty the cache tree. We just set the cache
-  pointer to NULL here, because we cannot release the store at this stage. */
+/* If caching was disabled, empty the cache tree. We just set the cache
+pointer to NULL here, because we cannot release the store at this stage. */
 
   else
     {
@@ -629,6 +682,7 @@ else
     }
   }
 
+out:
 DEBUG(D_lookup)
   {
   if (data)
@@ -772,14 +826,14 @@ just in case the original key is too long for the string_sprintf() buffer (it
 else if (partial >= 0)
   {
   int len = Ustrlen(keystring);
-  uschar *keystring2;
+  uschar * keystring2;
 
   /* Try with the affix on the front, except for a zero-length affix */
 
   if (affixlen == 0) keystring2 = keystring; else
     {
     keystring2 = store_get(len + affixlen + 1,
-                       is_tainted(keystring) || is_tainted(affix));
+         is_tainted(keystring) || is_tainted(affix) ? GET_TAINTED : GET_UNTAINTED);
     Ustrncpy(keystring2, affix, affixlen);
     Ustrcpy(keystring2 + affixlen, keystring);
     DEBUG(D_lookup) debug_printf_indent("trying partial match %s\n", keystring2);
@@ -793,20 +847,20 @@ else if (partial >= 0)
   if (!yield)
     {
     int dotcount = 0;
-    uschar *keystring3 = keystring2 + affixlen;
-    uschar *s = keystring3;
-    while (*s != 0) if (*s++ == '.') dotcount++;
+    uschar * keystring3 = keystring2 + affixlen;
+
+    for(uschar * s = keystring3; *s; ) if (*s++ == '.') dotcount++;
 
     while (dotcount-- >= partial)
       {
-      while (*keystring3 != 0 && *keystring3 != '.') keystring3++;
+      while (*keystring3 && *keystring3 != '.') keystring3++;
 
       /* If we get right to the end of the string (which will be the last time
       through this loop), we've failed if the affix is null. Otherwise do one
       last lookup for the affix itself, but if it is longer than 1 character,
       remove the last character if it is ".". */
 
-      if (*keystring3 == 0)
+      if (!*keystring3)
         {
         if (affixlen < 1) break;
         if (affixlen > 1 && affix[affixlen-1] == '.') affixlen--;
@@ -827,7 +881,8 @@ else if (partial >= 0)
       if (yield)
         {
         /* First variable is the wild part; second is the fixed part. Take care
-        to get it right when keystring3 is just "*". */
+        to get it right when keystring3 is just "*".  Return a de-tainted version
+       of the fixed part, on the grounds it has been validated by the lookup. */
 
         if (expand_setup && *expand_setup >= 0)
           {
@@ -837,8 +892,10 @@ else if (partial >= 0)
           expand_nstring[*expand_setup] = keystring;
           expand_nlength[*expand_setup] = wildlength;
           *expand_setup += 1;
-          expand_nstring[*expand_setup] = keystring + wildlength + 1;
-          expand_nlength[*expand_setup] = (fixedlength < 0)? 0 : fixedlength;
+         if (fixedlength < 0) fixedlength = 0;
+          expand_nstring[*expand_setup] = string_copyn_taint(
+           keystring + wildlength + 1, fixedlength, GET_UNTAINTED);
+          expand_nlength[*expand_setup] = fixedlength;
           }
         break;
         }
@@ -856,10 +913,10 @@ is set to the string to the left of the @. */
 if (!yield  &&  starflags & SEARCH_STARAT)
   {
   uschar *atat = Ustrrchr(keystring, '@');
-  if (atat != NULL && atat > keystring)
+  if (atat && atat > keystring)
     {
     int savechar;
-    savechar = *(--atat);
+    savechar = *--atat;
     *atat = '*';
 
     DEBUG(D_lookup) debug_printf_indent("trying default match %s\n", atat);
@@ -903,16 +960,19 @@ complete non-wild domain entry, or we matched a wild-carded entry without
 chopping off any of the domain components, set up the expansion variables
 (if required) so that the first one is empty, and the second one is the
 fixed part of the domain. The set_null_wild flag is set only when yield is not
-NULL. */
+NULL.  Return a de-tainted version of the fixed part, on the grounds it has been
+validated by the lookup. */
 
 if (set_null_wild && expand_setup && *expand_setup >= 0)
   {
+  int fixedlength = Ustrlen(keystring);
   *expand_setup += 1;
   expand_nstring[*expand_setup] = keystring;
   expand_nlength[*expand_setup] = 0;
   *expand_setup += 1;
-  expand_nstring[*expand_setup] = keystring;
-  expand_nlength[*expand_setup] = Ustrlen(keystring);
+  expand_nstring[*expand_setup] = string_copyn_taint(
+           keystring, fixedlength, GET_UNTAINTED);
+  expand_nlength[*expand_setup] = fixedlength;
   }
 
 /* If we have a result, check the options to see if the key was wanted rather
@@ -920,7 +980,7 @@ than the result.  Return a de-tainted version of the key on the grounds that
 it have been validated by the lookup. */
 
 if (yield && ret_key)
-  yield = string_copy_taint(keystring, FALSE);
+  yield = string_copy_taint(keystring, GET_UNTAINTED);
 
 return yield;
 }