From: Willy Tarreau <w@1wt.eu>
Date: Sun, 19 Jun 2022 14:49:51 +0000 (+0200)
Subject: MEDIUM: debug: detect redefinition of symbols upon dlopen()
X-Git-Tag: v2.7-dev1~28
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=177aed56dcb38e03f8d1ab5cb9e5a3fd2254c22f;p=thirdparty%2Fhaproxy.git

MEDIUM: debug: detect redefinition of symbols upon dlopen()

In order to better detect the danger caused by extra shared libraries
which replace some symbols, upon dlopen() we now compare a few critical
symbols such as malloc(), free(), and some OpenSSL symbols, to see if
the loaded library comes with its own version. If this happens, a
warning is emitted and TAINTED_REDEFINITION is set. This is important
because some external libs might be linked against different libraries
than the ones haproxy was linked with, and most often this will end
very badly (e.g. an OpenSSL object is allocated by haproxy and freed
by such libs).

Since the main source of dlopen() calls is the Lua lib, via a "require"
statement, it's worth trying to show a Lua call trace when detecting a
symbol redefinition during dlopen(). As such we emit a Lua backtrace if
Lua is detected as being in use.
---

diff --git a/include/haproxy/bug.h b/include/haproxy/bug.h
index a4980af8a9..ce83536ea2 100644
--- a/include/haproxy/bug.h
+++ b/include/haproxy/bug.h
@@ -201,6 +201,7 @@ enum tainted_flags {
 	TAINTED_WARN                   = 0x00000010, /* a WARN_ON triggered */
 	TAINTED_BUG                    = 0x00000020, /* a BUG_ON triggered */
 	TAINTED_SHARED_LIBS            = 0x00000040, /* a shared library was loaded */
+	TAINTED_REDEFINITION           = 0x00000080, /* symbol redefinition detected */
 };
 
 /* this is a bit field made of TAINTED_*, and is declared in haproxy.c */
diff --git a/src/tools.c b/src/tools.c
index 5a35027502..c2ee7c953c 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -5810,7 +5810,21 @@ int openssl_compare_current_name(const char *name)
 void *dlopen(const char *filename, int flags)
 {
 	static void *(*_dlopen)(const char *filename, int flags);
+	struct {
+		const char *name;
+		void *curr, *next;
+	} check_syms[] = {
+		{ .name = "malloc", },
+		{ .name = "free", },
+		{ .name = "SSL_library_init", },
+		{ .name = "X509_free", },
+		/* insert only above, 0 must be the last one */
+		{ 0 },
+	};
+	const char *trace;
+	void *addr;
 	void *ret;
+	int sym = 0;
 
 	if (!_dlopen) {
 		_dlopen = get_sym_next_addr("dlopen");
@@ -5820,6 +5834,16 @@ void *dlopen(const char *filename, int flags)
 		}
 	}
 
+	/* save a few pointers to critical symbols. We keep a copy of both the
+	 * current and the next value, because we might already have replaced
+	 * some of them (e.g. malloc/free with DEBUG_MEM_STATS), and we're only
+	 * interested in verifying that a loaded library doesn't come with a
+	 * completely different definition that would be incompatible.
+	 */
+	for (sym = 0; check_syms[sym].name; sym++) {
+		check_syms[sym].curr = get_sym_curr_addr(check_syms[sym].name);
+		check_syms[sym].next = get_sym_next_addr(check_syms[sym].name);
+	}
 
 	/* now open the requested lib */
 	ret = _dlopen(filename, flags);
@@ -5828,6 +5852,25 @@ void *dlopen(const char *filename, int flags)
 
 	mark_tainted(TAINTED_SHARED_LIBS);
 
+	/* and check that critical symbols didn't change */
+	for (sym = 0; check_syms[sym].name; sym++) {
+		if (!check_syms[sym].curr && !check_syms[sym].next)
+			continue;
+
+		addr = dlsym(ret, check_syms[sym].name);
+		if (!addr || addr == check_syms[sym].curr || addr == check_syms[sym].next)
+			continue;
+
+		/* OK it's clear that this symbol was redefined */
+		mark_tainted(TAINTED_REDEFINITION);
+
+		trace = hlua_show_current_location("\n    ");
+		ha_warning("dlopen(): shared library '%s' brings a different definition of symbol '%s'. The process cannot be trusted anymore!%s%s\n",
+		           filename, check_syms[sym].name,
+		           trace ? " Suspected call location: \n    " : "",
+		           trace ? trace : "");
+	}
+
 	return ret;
 }
 #endif