Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libproxy crashes in connection with glibproxyresolver #199

Closed
cal0pteryx opened this issue Jan 18, 2023 · 11 comments
Closed

libproxy crashes in connection with glibproxyresolver #199

cal0pteryx opened this issue Jan 18, 2023 · 11 comments

Comments

@cal0pteryx
Copy link

Additional infos: The client program is using pyGObject (and libsoup), and it receives SIGABRT shortly after starting. There is no explicit get_proxies call coming from the client using pyGObject. The client used to work without this issue, but at some point dependencies were upgraded, and then these errors occurred (also for older versions of the client).

Here is an excerpt of the latest report from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028192

(gdb) bt
#0  _Unwind_GetRegionStart (context=0x0) at ../../../src/libgcc/unwind-dw2.c:384
#1  0x00007ffff2ca8689 in read_encoded_value (val=0x7fffcf3a1eb8, p=0x7fffcf3a279e "", encoding=<optimized out>, context=0x0) at /build/gcc-12-PBog5r/gcc-12-12.2.0/src/libstdc++-v3/../libgcc/unwind-pe.h:304
#2  __cxxabiv1::__gxx_personality_v0(int, _Unwind_Action, _Unwind_Exception_Class, _Unwind_Exception*, _Unwind_Context*) (version=<optimized out>, actions=2, exception_class=5138137972254386944, ue_header=<optimized out>, context=0x7fffcf3a2380) at ../../../../src/libstdc++-v3/libsupc++/eh_personality.cc:503
#3  0x00007ffff0994131 in _Unwind_Phase2 (context=0x7fffcf3a2380, exception_object=0x7fffc0002e10) at unwind/unwind-internal.h:118
#4  _Unwind_Resume (exception_object=0x7fffc0002e10) at unwind/Resume.c:37
#5  0x00007fffcd0f834a in __gnu_cxx::new_allocator<libproxy::url>::~new_allocator() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/11/ext/new_allocator.h:89
#6  std::allocator<libproxy::url>::~allocator() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/11/bits/allocator.h:174
#7  std::_Vector_base<libproxy::url, std::allocator<libproxy::url> >::_Vector_impl::~_Vector_impl() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/11/bits/stl_vector.h:128
#8  std::_Vector_base<libproxy::url, std::allocator<libproxy::url> >::~_Vector_base() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/11/bits/stl_vector.h:337
#9  std::vector<libproxy::url, std::allocator<libproxy::url> >::~vector() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/11/bits/stl_vector.h:683
#10 envvar_config_extension::get_config(libproxy::url const&) (this=<optimized out>, dst=<optimized out>) at ./libproxy/modules/config_envvar.cpp:60
#11 0x00007fffcd0eee52 in libproxy::proxy_factory::get_config(libproxy::url&, std::vector<libproxy::url, std::allocator<libproxy::url> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (this=0x555557b3f6d0, realurl=..., config=std::vector of length 0, capacity 0, ignore="") at ./libproxy/proxy.cpp:265
#12 0x00007fffcd0ef287 in libproxy::proxy_factory::get_proxies(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (this=0x555557b3f6d0, realurl="none://server.domain:443") at ./libproxy/proxy.cpp:206
#13 0x00007fffcd0ef751 in px_proxy_factory_get_proxies(pxProxyFactory_*, char const*) (self=0x555557b3f6d0, url=url@entry=0x555557576260 "none://server.domain:443") at ./libproxy/proxy.cpp:465
#14 0x00007fffcd96d61f in get_libproxy_proxies (task=0x555557baa6f0 [GTask], source_object=0x5555573ce960, task_data=0x555557576260, cancellable=<optimized out>) at ../proxy/libproxy/glibproxyresolver.c:153
#15 0x00007ffff6cec793 in g_task_thread_pool_thread (thread_data=0x555557baa6f0, pool_data=<optimized out>) at ../../../gio/gtask.c:1454
#16 0x00007ffff6fc96da in g_thread_pool_thread_proxy (data=<optimized out>) at ../../../glib/gthreadpool.c:352
#17 0x00007ffff6fc8d0d in g_thread_proxy (data=0x55555729ea40) at ../../../glib/gthread.c:831
#18 0x00007ffff7d2efd4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
#19 0x00007ffff7daf66c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
(gdb) 

-- System Information:
Debian Release: bookworm/sid
APT prefers testing
APT policy: (500, 'testing'), (90, 'unstable'), (10, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 6.0.0-6-amd64 (SMP w/2 CPU threads; PREEMPT)
Kernel taint flags: TAINT_FIRMWARE_WORKAROUND, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages gajim depends on:
ii desktop-file-utils 0.26-1
ii gir1.2-gst-plugins-base-1.0 1.20.5-1
ii gir1.2-gtk-3.0 3.24.36-1
ii gir1.2-gtksource-4 4.8.4-4
ii python3 3.10.6-3+b1
ii python3-cairo 1.20.1-5
ii python3-cryptography 38.0.4-2
ii python3-css-parser 1.0.8-1
ii python3-gi 3.42.2-3
ii python3-gi-cairo 3.42.2-3
ii python3-idna 3.3-1
ii python3-keyring 23.9.3-2
ii python3-nbxmpp 4.0.1-1
ii python3-packaging 22.0-2
ii python3-pil 9.4.0-1+b1
ii python3-precis-i18n 1.0.5-1

Versions of packages gajim recommends:
ii alsa-utils 1.2.8-1
ii aspell-en [aspell-dictionary] 2018.04.16-0-1
ii ca-certificates 20211016
ii dbus 1.14.4-1
ii fonts-noto-color-emoji 2.038-1
ii gajim-omemo 2.9.0-1
pn gajim-openpgp
ii gir1.2-farstream-0.2 0.2.9-1
pn gir1.2-geoclue-2.0
ii gir1.2-gsound-1.0 1.0.3-2
ii gir1.2-gspell-1 1.12.0-1+b1
ii gir1.2-gstreamer-1.0 1.20.5-1
pn gir1.2-gupnpigd-1.0
ii gir1.2-secret-1 0.20.5-3
ii gstreamer1.0-gl 1.20.5-1
ii gstreamer1.0-nice 0.1.18-2
ii gstreamer1.0-plugins-ugly 1.20.5-1
ii notification-daemon 3.20.0-4+b1
ii pulseaudio-utils 16.1+dfsg1-2+b1
ii python3-dbus 1.3.2-3
ii python3-gssapi 1.8.2-1
ii python3-sentry-sdk 1.9.10-2
ii sox 14.4.2+git20190427-3+b1
ii xfce4-notifyd [notification-daemon] 0.6.5-1

Versions of packages gajim suggests:
ii libxss1 1:1.2.3-1
pn nautilus-sendto

-- no debconf information

@cal0pteryx
Copy link
Author

cal0pteryx commented Jan 19, 2023

People were able to bisect this, and found libproxy 0.4.15-15 (Debian package) as the last known working version. Seems the issue was introduced by libproxy 0.4.16-1 (Debian package).

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028638

@Vogtinator
Copy link
Contributor

That looks very strange. Apparently __gnu_cxx::new_allocator<libproxy::url>::~new_allocator() throws an exception? I don't see how that can happen.

@sre
Copy link

sre commented Jan 19, 2023

Hi,

I did some tests and the crash happens when the runtime error is thrown in this place: https://github.com/libproxy/libproxy/blob/master/libproxy/modules/config_envvar.cpp#L56 . With the following patch applied I no longer see a crash. Note, that I haven't spent the time to fully understand the code base. I don't know if its ok to return an empty vector when no environment variables are set.

diff -Nur libproxy/modules/config_envvar.cpp.orig libproxy/modules/config_envvar.cpp
--- libproxy/modules/config_envvar.cpp.orig     2023-01-19 23:28:33.700164562 +0100
+++ libproxy/modules/config_envvar.cpp  2023-01-19 23:27:40.715770961 +0100
@@ -52,10 +52,9 @@
                                proxy = getenv("HTTP_PROXY");
                }

-               if (!proxy)
-                       throw runtime_error("Unable to read configuration");
+               if (proxy)
+                       response.push_back(url(proxy));

-                response.push_back(url(proxy));
                return response;
        }

@Vogtinator
Copy link
Contributor

Is something built with -fno-exceptions by chance? Could also be that unwinding across library boundaries is broken.

@sre
Copy link

sre commented Jan 20, 2023

glib-networking uses px_proxy_factory_get_proxies(), which is in libproxy's shared object and handles the exception, so there is no unwinding happening across library boundaries. (sidenote: glib-networking is written in C and not in C++, so there are no exceptions). I did some more tests:

This works:

vector<url> get_config(const url &dst) {
	throw runtime_error("Unable to read configuration");
}

This works:

vector<url> get_config(const url &dst) {
	vector<url> response;
	response.push_back(url("direct://"));
	return response;
}

This results in a crash:

vector<url> get_config(const url &dst) {
	vector<url> response;
	response.push_back(url("direct://"));
	throw runtime_error("Unable to read configuration");
}

@sre
Copy link

sre commented Jan 20, 2023

I also did some investigation regarding compiler/linker flags. Debian's libproxy package currently enables all security hardening flags known by dpkg-buildflags. I was able to fix the crash by disabling the 'bindnow' hardening (which adds -Wl,-z,now to LDFLAGS).

@sre
Copy link

sre commented Jan 23, 2023

Building without bindnow (or any security flags for that matter) is not enough to properly fix the issue. Using the same libproxy binary on another system still resulted in the crash.

@sre
Copy link

sre commented Jan 23, 2023

It really looks like the problem is with the destructor of response when the runtime error is thrown. This fixes the issue on my laptop:

--- a/libproxy/modules/config_envvar.cpp	2022-06-20 16:23:35.000000000 +0200
+++ b/libproxy/modules/config_envvar.cpp	2023-01-23 21:37:57.439386306 +0100
@@ -26,12 +26,10 @@
 public:
 	vector<url> get_config(const url &dst) {
 		const char *proxy = NULL;
-                vector<url> response;
 
 		// If _PX_DEBUG_PACURL is set, use it as the PAC URL.
 		if (proxy = getenv("_PX_DEBUG_PACURL")) {
-			response.push_back(url(string("pac+") + proxy));
-			return response;
+			return std::vector<url> { url(string("pac+") + proxy) };
 		}
 
 		// If the URL is an ftp url, try to read the ftp proxy
@@ -55,8 +53,7 @@
 		if (!proxy)
 			throw runtime_error("Unable to read configuration");
 
-                response.push_back(url(proxy));
-		return response;
+		return std::vector<url> { url(proxy) };
 	}
 
 	string get_ignore(const url&) {

Interestingly I cannot reproduce the issue with the bundled utils/proxy.c tool, which also uses the C API.

@cal0pteryx
Copy link
Author

I can confirm that this patch seems to fix the issue. A fix in Debian's libproxy package (see https://sources.debian.org/patches/libproxy/0.4.18-1.2/ ) is in place, user feedback looks good so far.

@mcatanzaro
Copy link
Contributor

Well of course that patch does not really help us understand the bug here -- why is the allocator's destructor throwing? -- but it makes the code nicer, so it's good to do regardless. Feel free to submit a pull request.

@janbrummer
Copy link
Contributor

Closing as obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants