From f42deca923414cedcbb6d6646afbef460f50080c Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Fri, 6 May 2016 13:07:18 +0100
Subject: [PATCH] Avoid exposing passwords in log, on failing ldap lookup
 expansion.  Bug 165

---
 doc/doc-txt/ChangeLog |  2 +-
 src/src/deliver.c     | 20 +++-----------------
 src/src/expand.c      | 23 +++++++++++++++++++++++
 src/src/functions.h   |  1 +
 src/src/rewrite.c     | 17 +----------------
 src/src/route.c       | 17 ++---------------
 6 files changed, 31 insertions(+), 49 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 4171d61c4..3bf6fc908 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -27,7 +27,7 @@ JH/05 If main configuration option tls_certificate is unset, generate a
       selfsigned certificate for inbound TLS connections.
 
 JH/06 Bug 165: hide more cases of password exposure - this time in expansions
-      in rewrites.
+      in rewrites and routers.
 
 
 Exim version 4.87
diff --git a/src/src/deliver.c b/src/src/deliver.c
index c6de1b901..743fc83e8 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -1076,23 +1076,9 @@ malformed, it won't ever have gone near LDAP.) */
 if (addr->message)
   {
   const uschar * s = string_printing(addr->message);
-  if (s != addr->message)
-    addr->message = US s;
-    /* deconst cast ok as string_printing known to have alloc'n'copied */
-  if (  (  Ustrstr(s, "failed to expand") != NULL
-	|| Ustrstr(s, "expansion of ")    != NULL
-	)
-     && (  Ustrstr(s, "mysql")   != NULL
-        || Ustrstr(s, "pgsql")   != NULL
-	|| Ustrstr(s, "redis")   != NULL
-	|| Ustrstr(s, "sqlite")  != NULL
-	|| Ustrstr(s, "ldap:")   != NULL
-	|| Ustrstr(s, "ldaps:")  != NULL
-	|| Ustrstr(s, "ldapi:")  != NULL
-	|| Ustrstr(s, "ldapdn:") != NULL
-	|| Ustrstr(s, "ldapm:")  != NULL
-     )  )
-    addr->message = US"Temporary internal error";
+
+  /* deconst cast ok as string_printing known to have alloc'n'copied */
+  addr->message = expand_hide_passwords(US s);
   }
 
 /* If we used a transport that has one of the "return_output" options set, and
diff --git a/src/src/expand.c b/src/src/expand.c
index fbbc56316..f783682b4 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -7631,6 +7631,29 @@ return OK;
 
 
 
+/* Avoid potentially exposing a password in a string about to be logged */
+
+uschar *
+expand_hide_passwords(uschar * s)
+{
+return (  (  Ustrstr(s, "failed to expand") != NULL
+	  || Ustrstr(s, "expansion of ")    != NULL
+	  ) 
+       && (  Ustrstr(s, "mysql")   != NULL
+	  || Ustrstr(s, "pgsql")   != NULL
+	  || Ustrstr(s, "redis")   != NULL
+	  || Ustrstr(s, "sqlite")  != NULL
+	  || Ustrstr(s, "ldap:")   != NULL
+	  || Ustrstr(s, "ldaps:")  != NULL
+	  || Ustrstr(s, "ldapi:")  != NULL
+	  || Ustrstr(s, "ldapdn:") != NULL
+	  || Ustrstr(s, "ldapm:")  != NULL
+       )  ) 
+  ? US"Temporary internal error" : s;
+}
+
+
+
 
 /*************************************************
 **************************************************
diff --git a/src/src/functions.h b/src/src/functions.h
index 454037fe1..f690e9fd9 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -180,6 +180,7 @@ extern int     exp_bool(address_item *addr,
 extern BOOL    expand_check_condition(uschar *, uschar *, uschar *);
 extern uschar *expand_string(uschar *);	/* public, cannot make const */
 extern const uschar *expand_cstring(const uschar *); /* ... so use this one */
+extern uschar *expand_hide_passwords(uschar * );
 extern uschar *expand_string_copy(const uschar *);
 extern int_eximarith_t expand_string_integer(uschar *, BOOL);
 extern void    modify_variable(uschar *, void *);
diff --git a/src/src/rewrite.c b/src/src/rewrite.c
index f2a7ff273..830d2bb8d 100644
--- a/src/src/rewrite.c
+++ b/src/src/rewrite.c
@@ -206,22 +206,7 @@ for (rule = rewrite_rules;
     if (expand_string_forcedfail)
       { if ((rule->flags & rewrite_quit) != 0) break; else continue; }
 
-    /* Avoid potentially exposing a password */
-
-    if (  (  Ustrstr(expand_string_message, "failed to expand") != NULL
-	  || Ustrstr(expand_string_message, "expansion of ")    != NULL
-	  )
-       && (  Ustrstr(expand_string_message, "mysql")   != NULL
-	  || Ustrstr(expand_string_message, "pgsql")   != NULL
-	  || Ustrstr(expand_string_message, "redis")   != NULL
-	  || Ustrstr(expand_string_message, "sqlite")  != NULL
-	  || Ustrstr(expand_string_message, "ldap:")   != NULL
-	  || Ustrstr(expand_string_message, "ldaps:")  != NULL
-	  || Ustrstr(expand_string_message, "ldapi:")  != NULL
-	  || Ustrstr(expand_string_message, "ldapdn:") != NULL
-	  || Ustrstr(expand_string_message, "ldapm:")  != NULL
-       )  )
-      expand_string_message = US"Temporary internal error";
+    expand_string_message = expand_hide_passwords(expand_string_message);
 
     log_write(0, LOG_MAIN|LOG_PANIC, "Expansion of %s failed while rewriting: "
       "%s", rule->replacement, expand_string_message);
diff --git a/src/src/route.c b/src/src/route.c
index b289b0f34..f7a532567 100644
--- a/src/src/route.c
+++ b/src/src/route.c
@@ -1897,21 +1897,8 @@ if (unseen && r->next)
 /* Unset the address expansions, and return the final result. */
 
 ROUTE_EXIT:
-if (  yield == DEFER
-   && addr->message
-   && (  Ustrstr(addr->message, "failed to expand") != NULL
-      || Ustrstr(addr->message, "expansion of ") != NULL
-      )
-   && (  Ustrstr(addr->message, "mysql") != NULL
-      || Ustrstr(addr->message, "pgsql") != NULL
-      || Ustrstr(addr->message, "redis") != NULL
-      || Ustrstr(addr->message, "sqlite") != NULL
-      || Ustrstr(addr->message, "ldap:") != NULL
-      || Ustrstr(addr->message, "ldapdn:") != NULL
-      || Ustrstr(addr->message, "ldapm:") != NULL
-      )
-   )
-  addr->message = string_sprintf("Temporary internal error");
+if (yield == DEFER && addr->message)
+  addr->message = expand_hide_passwords(addr->message);
 
 deliver_set_expansions(NULL);
 router_name = NULL;
-- 
2.30.2