Taint: hybrid checking mode

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index a15e5b4a0b5241f05328cdd6620cd20b579053f1..f0dccdc62943f9abfbc764bf8530191eab5da30b 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -100,6 +100,14 @@ JH/21 Bug 2501: Fix init call in the heimdal authenticator.  Previously it
       buffer was in use at the time.  Change to a compile-time increase in the
       buffer size, when this authenticator is compiled into exim.
 
+JH/22 Taint checking: move to a hybrid approach for checking.  Previously, one
+      of two ways was used, depending on a build-time flag.  The fast method
+      relied on assumptions about the OS and libc malloc, which were known to
+      not hold for the BSD-derived platforms, and discovered to not hold for
+      32-bit Linux either.  In fact the glibc documentation describes cases
+      where these assumptions do not hold.  The new implementation tests for
+      the situation arising and actively switches over from fast to safe mode.
+
 
 Exim version 4.93
 -----------------

diff --git a/src/OS/Makefile-Linux b/src/OS/Makefile-Linux
index d1d501376afc48bb45d8462e5faf25f9a8266bb7..ae9f249a9abd896b1ac450524e3d9853cdf1ada5 100644 (file)
--- a/src/OS/Makefile-Linux
+++ b/src/OS/Makefile-Linux
@@ -18,9 +18,6 @@ CC=cc
 CFLAGS ?= -O -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
 CFLAGS_DYNAMIC ?= -shared -rdynamic
 
-# We have mmap/malloc addr spaces separate
-CFLAGS += -DTAINT_CHECK_FAST
-
 DBMLIB = -ldb
 USE_DB = yes
 

diff --git a/src/exim_monitor/em_version.c b/src/exim_monitor/em_version.c
index 52c55a4a3593b4b2438f768d2e6d9957c4f19013..9b9c7d4171e1b88f14c6a012a8351e875d9c02cd 100644 (file)
--- a/src/exim_monitor/em_version.c
+++ b/src/exim_monitor/em_version.c
@@ -5,6 +5,8 @@
 /* Copyright (c) University of Cambridge 1995 - 2018 */
 /* See the file NOTICE for conditions of use and distribution. */
 
+#define EM_VERSION_C
+
 #include "mytypes.h"
 #include "store.h"
 #include "macros.h"

diff --git a/src/src/functions.h b/src/src/functions.h
index 7677a3cd95e390e4ac32252477b40c94d6945aa3..2a2c0dbb8f9fa5bc799253394f525f0c450712e2 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -189,6 +189,7 @@ extern void    deliver_succeeded(address_item *);
 extern uschar *deliver_get_sender_address (uschar *id);
 extern void    delivery_re_exec(int);
 
+extern void    die_tainted(const uschar *, const uschar *, int);
 extern BOOL    directory_make(const uschar *, const uschar *, int, BOOL);
 #ifndef DISABLE_DKIM
 extern uschar *dkim_exim_query_dns_txt(const uschar *);
@@ -606,6 +607,61 @@ extern BOOL    write_chunk(transport_ctx *, uschar *, int);
 extern ssize_t write_to_fd_buf(int, const uschar *, size_t);
 
 
+/******************************************************************************/
+/* Predicate: if an address is in a tainted pool.
+By extension, a variable pointing to this address is tainted.
+*/
+
+static inline BOOL
+is_tainted(const void * p)
+{
+#if defined(COMPILE_UTILITY) || defined(MACRO_PREDEF) || defined(EM_VERSION_C)
+return FALSE;
+
+#else
+extern BOOL is_tainted_fn(const void *);
+extern void * tainted_base, * tainted_top;
+
+return f.taint_check_slow
+  ? is_tainted_fn(p) : p >= tainted_base && p < tainted_top;
+#endif
+}
+
+/******************************************************************************/
+/* String functions */
+static inline uschar * __Ustrcat(uschar * dst, const uschar * src, const char * func, int line)
+{
+#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
+if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrcat", CUS func, line);
+#endif
+return US strcat(CS dst, CCS src);
+}
+static inline uschar * __Ustrcpy(uschar * dst, const uschar * src, const char * func, int line)
+{
+#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
+if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrcpy", CUS func, line);
+#endif
+return US strcpy(CS dst, CCS src);
+}
+static inline uschar * __Ustrncat(uschar * dst, const uschar * src, size_t n, const char * func, int line)
+{
+#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
+if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrncat", CUS func, line);
+#endif
+return US strncat(CS dst, CCS src, n);
+}
+static inline uschar * __Ustrncpy(uschar * dst, const uschar * src, size_t n, const char * func, int line)
+{
+#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
+if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrncpy", CUS func, line);
+#endif
+return US strncpy(CS dst, CCS src, n);
+}
+/*XXX will likely need unchecked copy also */
+
+
+/******************************************************************************/
+
 #if !defined(MACRO_PREDEF) && !defined(COMPILE_UTILITY)
 /* exim_chown - in some NFSv4 setups *seemes* to be an issue with
 chown(<exim-uid>, <exim-gid>).
@@ -638,8 +694,8 @@ exim_chown(const uschar *name, uid_t owner, gid_t group)
 return chown(CCS name, owner, group)
   ? exim_chown_failure(-1, name, owner, group) : 0;
 }
-
 #endif /* !MACRO_PREDEF && !COMPILE_UTILITY */
+
 /******************************************************************************/
 /* String functions */
 

diff --git a/src/src/globals.c b/src/src/globals.c
index edd1edbbc9eb40fe78a6b19403bd48136c75d78c..f16e19bc534f8851742334eb45fb5ebd65db300a 100644 (file)
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -312,6 +312,7 @@ struct global_flags f =
        .synchronous_delivery   = FALSE,
        .system_filtering       = FALSE,
 
+       .taint_check_slow       = FALSE,
        .tcp_fastopen_ok        = FALSE,
        .tcp_in_fastopen        = FALSE,
        .tcp_in_fastopen_data   = FALSE,

diff --git a/src/src/globals.h b/src/src/globals.h
index 9dfa4a7689e6af2f9d4a98f4d82b35c7e224edd9..74af185acff6b9d2e4df7bc83b5842a278660fed 100644 (file)
--- a/src/src/globals.h
+++ b/src/src/globals.h
@@ -274,6 +274,7 @@ extern struct global_flags {
  BOOL   synchronous_delivery           :1; /* TRUE if -odi is set */
  BOOL   system_filtering               :1; /* TRUE when running system filter */
 
+ BOOL   taint_check_slow               :1; /* malloc/mmap are not returning distinct ranges */
  BOOL   tcp_fastopen_ok                        :1; /* appears to be supported by kernel */
  BOOL   tcp_in_fastopen                        :1; /* conn usefully used fastopen */
  BOOL   tcp_in_fastopen_data           :1; /* fastopen carried data */

diff --git a/src/src/mytypes.h b/src/src/mytypes.h
index d652daea3637a1ff3382882456d9efa7daa3894f..6d2f169b7fc342f9a68679cb87277322bb9e7cc5 100644 (file)
--- a/src/src/mytypes.h
+++ b/src/src/mytypes.h
@@ -94,28 +94,24 @@ functions that are called quite often; for other calls to external libraries
 #define Ulstat(s,t)        lstat(CCS(s),t)
 
 #ifdef O_BINARY                                                        /* This is for Cygwin,  */
-#define Uopen(s,n,m)       exim_open(CCS(s),(n)|O_BINARY,m)    /* where all files must */
-#define Uopen2(s,n)        exim_open2(CCS(s),(n)|O_BINARY)
+# define Uopen(s,n,m)       exim_open(CCS(s),(n)|O_BINARY,m)   /* where all files must */
+# define Uopen2(s,n)        exim_open2(CCS(s),(n)|O_BINARY)
 #else                                                          /* be opened as binary  */
-#define Uopen(s,n,m)       exim_open(CCS(s),n,m)               /* to avoid problems    */
-#define Uopen2(s,n)        exim_open2(CCS(s),n)        
+# define Uopen(s,n,m)       exim_open(CCS(s),n,m)              /* to avoid problems    */
+# define Uopen2(s,n)        exim_open2(CCS(s),n)       
 #endif                                                         /* with CRLF endings.   */
 #define Uread(f,b,l)       read(f,CS(b),l)
 #define Urename(s,t)       rename(CCS(s),CCS(t))
 #define Ustat(s,t)         stat(CCS(s),t)
-#define Ustrcat(s,t)       __Ustrcat(s, CUS(t), __FUNCTION__, __LINE__)
 #define Ustrchr(s,n)       US strchr(CCS(s),n)
 #define CUstrchr(s,n)      CUS strchr(CCS(s),n)
 #define CUstrerror(n)      CUS strerror(n)
 #define Ustrcmp(s,t)       strcmp(CCS(s),CCS(t))
-#define Ustrcpy(s,t)       __Ustrcpy(s, CUS(t), __FUNCTION__, __LINE__)
 #define Ustrcpy_nt(s,t)    strcpy(CS s, CCS t)         /* no taint check */
 #define Ustrcspn(s,t)      strcspn(CCS(s),CCS(t))
 #define Ustrftime(s,m,f,t) strftime(CS(s),m,f,t)
 #define Ustrlen(s)         (int)strlen(CCS(s))
-#define Ustrncat(s,t,n)    __Ustrncat(s, CUS(t),n, __FUNCTION__, __LINE__)
 #define Ustrncmp(s,t,n)    strncmp(CCS(s),CCS(t),n)
-#define Ustrncpy(s,t,n)    __Ustrncpy(s, CUS(t),n, __FUNCTION__, __LINE__)
 #define Ustrncpy_nt(s,t,n) strncpy(CS s, CCS t, n)     /* no taint check */
 #define Ustrpbrk(s,t)      strpbrk(CCS(s),CCS(t))
 #define Ustrrchr(s,n)      US strrchr(CCS(s),n)
@@ -128,57 +124,17 @@ functions that are called quite often; for other calls to external libraries
 #define Ustrtoul(s,t,b)    strtoul(CCS(s),CSS(t),b)
 #define Uunlink(s)         unlink(CCS(s))
 
-extern void die_tainted(const uschar *, const uschar *, int);
-
-/* Predicate: if an address is in a tainted pool.
-By extension, a variable pointing to this address is tainted.
-*/
-
-static inline BOOL
-is_tainted(const void * p)
-{
-#if defined(COMPILE_UTILITY) || defined(MACRO_PREDEF)
-return FALSE;
-
-#elif !defined(TAINT_CHECK_FAST)
-extern BOOL is_tainted_fn(const void *);
-return is_tainted_fn(p);
-
+#ifdef EM_VERSION_C
+# define Ustrcat(s,t)       strcat(CS(s), CCS(t))
+# define Ustrcpy(s,t)       strcpy(CS(s), CCS(t))
+# define Ustrncat(s,t,n)    strncat(CS(s), CCS(t), n)
+# define Ustrncpy(s,t,n)    strncpy(CS(s), CCS(t), n)
 #else
-extern void * tainted_base, * tainted_top;
-return p >= tainted_base && p < tainted_top;
-#endif
-}
-
-static inline uschar * __Ustrcat(uschar * dst, const uschar * src, const char * func, int line)
-{
-#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
-if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrcat", CUS func, line);
-#endif
-return US strcat(CS dst, CCS src);
-}
-static inline uschar * __Ustrcpy(uschar * dst, const uschar * src, const char * func, int line)
-{
-#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
-if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrcpy", CUS func, line);
-#endif
-return US strcpy(CS dst, CCS src);
-}
-static inline uschar * __Ustrncat(uschar * dst, const uschar * src, size_t n, const char * func, int line)
-{
-#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
-if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrncat", CUS func, line);
-#endif
-return US strncat(CS dst, CCS src, n);
-}
-static inline uschar * __Ustrncpy(uschar * dst, const uschar * src, size_t n, const char * func, int line)
-{
-#if !defined(COMPILE_UTILITY) && !defined(MACRO_PREDEF)
-if (!is_tainted(dst) && is_tainted(src)) die_tainted(US"Ustrncpy", CUS func, line);
+# define Ustrcat(s,t)       __Ustrcat(s, CUS(t), __FUNCTION__, __LINE__)
+# define Ustrcpy(s,t)       __Ustrcpy(s, CUS(t), __FUNCTION__, __LINE__)
+# define Ustrncat(s,t,n)    __Ustrncat(s, CUS(t), n, __FUNCTION__, __LINE__)
+# define Ustrncpy(s,t,n)    __Ustrncpy(s, CUS(t), n, __FUNCTION__, __LINE__)
 #endif
-return US strncpy(CS dst, CCS src, n);
-}
-/*XXX will likely need unchecked copy also */
 
 #endif
 /* End of mytypes.h */

diff --git a/src/src/store.c b/src/src/store.c
index 61f9464af58bdd76b42d07529a05cce8bc78001c..aceb0e5d66789d50f128698e3b0178407de75343 100644 (file)
--- a/src/src/store.c
+++ b/src/src/store.c
@@ -186,7 +186,6 @@ static void   internal_tainted_free(storeblock *, const char *, int linenumber);
 
 /******************************************************************************/
 
-#ifndef TAINT_CHECK_FAST
 /* Test if a pointer refers to tainted memory.
 
 Slower version check, for use when platform intermixes malloc and mmap area
@@ -205,19 +204,18 @@ int pool;
 for (pool = POOL_TAINT_BASE; pool < nelem(chainbase); pool++)
   if ((b = current_block[pool]))
     {
-    char * bc = CS b + ALIGNED_SIZEOF_STOREBLOCK;
-    if (CS p >= bc && CS p <= bc + b->length) return TRUE;
+    uschar * bc = US b + ALIGNED_SIZEOF_STOREBLOCK;
+    if (US p >= bc && US p <= bc + b->length) return TRUE;
     }
 
 for (pool = POOL_TAINT_BASE; pool < nelem(chainbase); pool++)
   for (b = chainbase[pool]; b; b = b->next)
     {
-    char * bc = CS b + ALIGNED_SIZEOF_STOREBLOCK;
-    if (CS p >= bc && CS p <= bc + b->length) return TRUE;
+    uschar * bc = US b + ALIGNED_SIZEOF_STOREBLOCK;
+    if (US p >= bc && US p <= bc + b->length) return TRUE;
     }
 return FALSE;
 }
-#endif
 
 
 void
@@ -227,6 +225,13 @@ log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Taint mismatch, %s: %s %d\n",
        msg, func, line);
 }
 
+static void
+use_slow_taint_check(void)
+{
+DEBUG(D_any) debug_printf("switching to slow-mode taint checking\n");
+f.taint_check_slow = TRUE;
+}
+
 
 /*************************************************
 *       Get a block from the current pool        *
@@ -850,6 +855,14 @@ if (!(yield = malloc((size_t)size)))
   log_write(0, LOG_MAIN|LOG_PANIC_DIE, "failed to malloc %d bytes of memory: "
     "called from line %d in %s", size, linenumber, func);
 
+/* If malloc ever returns apparently tainted memory, which glibc
+malloc will as it uses mmap for larger requests, we must switch to
+the slower checking for tainting (checking an address against all
+the tainted pool block spans, rather than just the mmap span) */
+
+if (!f.taint_check_slow && is_tainted(yield))
+  use_slow_taint_check();
+
 return store_alloc_tail(yield, size, func, linenumber, US"Malloc");
 }