From 36eb5d3d77426d8cbf4243ea752f8d8cd1d5c682 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 16 Jan 2020 14:12:56 +0000 Subject: [PATCH] Taint: hybrid checking mode --- doc/doc-txt/ChangeLog | 8 ++++ src/OS/Makefile-Linux | 3 -- src/exim_monitor/em_version.c | 2 + src/src/functions.h | 58 ++++++++++++++++++++++++++++- src/src/globals.c | 1 + src/src/globals.h | 1 + src/src/mytypes.h | 70 +++++++---------------------------- src/src/store.c | 25 ++++++++++--- 8 files changed, 101 insertions(+), 67 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index a15e5b4a0..f0dccdc62 100644 --- 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 d1d501376..ae9f249a9 100644 --- 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 52c55a4a3..9b9c7d417 100644 --- 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 7677a3cd9..2a2c0dbb8 100644 --- 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(, ). @@ -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 edd1edbbc..f16e19bc5 100644 --- 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 9dfa4a768..74af185ac 100644 --- 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 d652daea3..6d2f169b7 100644 --- 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 61f9464af..aceb0e5d6 100644 --- 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"); } -- 2.30.2