From 1ccd26e24267ffa0c40b70c2c3282481fe4977c7 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 16 Jan 2020 14:12:56 +0000 Subject: [PATCH 22/22] Taint: hybrid checking mode (cherry picked from commit 36eb5d3d77426d8cbf4243ea752f8d8cd1d5c682) --- doc/ChangeLog | 8 +++++ exim_monitor/em_version.c | 2 ++ src/functions.h | 58 +++++++++++++++++++++++++++++++- src/globals.c | 1 + src/globals.h | 1 + src/mytypes.h | 62 +++++------------------------------ src/store.c | 40 +++++++++++++++------- 7 files changed, 107 insertions(+), 65 deletions(-) diff --git doc/ChangeLog doc/ChangeLog index f112fc9bf..508b8fa49 100644 --- doc/ChangeLog +++ doc/ChangeLog @@ -59,6 +59,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 exim_monitor/em_version.c exim_monitor/em_version.c index 52c55a4a3..9b9c7d417 100644 --- exim_monitor/em_version.c +++ 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 src/functions.h src/functions.h index 87d1a04d8..0b5905562 100644 --- src/functions.h +++ src/functions.h @@ -187,6 +187,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 *); @@ -602,6 +603,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(, ). @@ -634,8 +690,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 src/globals.c src/globals.c index 85a25a7f2..72449229e 100644 --- src/globals.c +++ src/globals.c @@ -311,6 +311,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 src/globals.h src/globals.h index ca342acc2..ac7bb8ef3 100644 --- src/globals.h +++ src/globals.h @@ -272,6 +272,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 src/mytypes.h src/mytypes.h index ceb9f1b55..e31ee8c1a 100644 --- src/mytypes.h +++ src/mytypes.h @@ -100,19 +100,15 @@ functions that are called quite often; for other calls to external libraries #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) @@ -125,57 +121,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_SLOW) -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 src/store.c src/store.c index a06e1c19a..692a993e9 100644 --- src/store.c +++ src/store.c @@ -162,8 +162,14 @@ static void internal_tainted_free(storeblock *, const char *, int linenumber); /******************************************************************************/ -/* Slower version check, for use when platform intermixes malloc and mmap area -addresses. */ +/* Test if a pointer refers to tainted memory. + +Slower version check, for use when platform intermixes malloc and mmap area +addresses. Test against the current-block of all tainted pools first, then all +blocks of all tainted pools. + +Return: TRUE iff tainted +*/ BOOL is_tainted_fn(const void * p) @@ -171,23 +177,20 @@ is_tainted_fn(const void * p) storeblock * b; int pool; -for (pool = 0; pool < nelem(chainbase); 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) goto hit; + uschar * bc = US b + ALIGNED_SIZEOF_STOREBLOCK; + if (US p >= bc && US p <= bc + b->length) return TRUE; } -for (pool = 0; pool < nelem(chainbase); pool++) +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) goto hit; + uschar * bc = US b + ALIGNED_SIZEOF_STOREBLOCK; + if (US p >= bc && US p <= bc + b->length) return TRUE; } return FALSE; - -hit: -return pool >= POOL_TAINT_BASE; } @@ -198,6 +201,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 * @@ -820,6 +830,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.24.1