Bugzilla – Bug 1157462
libdbus-1 uses abort() which is an absolute nogo for a system library
Last modified: 2022-02-08 08:23:43 UTC
Indeed: /local/werner> nm -D /usr/lib/libdbus-1.so.3.19.4 | grep abort 0003a4d0 T _dbus_abort U abort
Could one please explain why libdbus-1 is configured with default --enable-embedded-tests --enable-asserts instead of using --disable-embedded-tests --disable-asserts why we're using users systems as debugging systems? Any system library should not use abort() nor exist() for debugging.
@ Simon : I'd like to see an answer on this bug. Do you agree, that system libraries shiuld not use in debugging cases the abort() nor exit() calls at all? If you do not agree, then yoi might give some reason here why you think the libdbus-1 should do an abort() for nitpicking corner cases.
That is what Debian does https://sources.debian.org/patches/dbus/1.10.28-0+deb9u1/debian/Don-t-abort-on-fatal-warnings-by-default.patch/ By which rational we do not the same?
Are we affected by CVE-2019-12749?
I see bug#1137832 ... but is this also fixed in Factory/Tumbleweed with dbus-1.12.12?
AFAICS from https://dbus.freedesktop.org/releases/dbus/dbus-1.13.12.tar.xz the fix had been added upstream in dbus-1.13.12/dbus/dbus-auth.c also from Changelog dbus 1.13.12 (2019-06-11) ========================= The “patio squirrel” release. Security fixes: • CVE-2019-12749: Do not attempt to carry out DBUS_COOKIE_SHA1 authentication for identities that differ from the user running the DBusServer. Previously, a local attacker could manipulate symbolic links in their own home directory to bypass authentication and connect to a DBusServer with elevated privileges. The standard system and session dbus-daemons in their default configuration were immune to this attack because they did not allow DBUS_COOKIE_SHA1, but third-party users of DBusServer such as Upstart could be vulnerable. Thanks to Joe Vennix of Apple Information Security. (dbus#269, Simon McVittie) Enhancements: • dbus-daemon <allow> and <deny> rules can now specify a send_destination_prefix attribute, which is like a combination of send_destination and the arg0namespace keyword in match rules: a rule with send_destination_prefix="com.example.Foo" matches messages sent to any destination that is in the queue to own well-known names like com.example.Foo or com.example.Foo.A.B (but not com.example.Foobar). (dbus!85, Adrian Szyndela)
Similar with stable realease 1.12.16 * CVE-2019-12749: Do not attempt to carry out DBUS_COOKIE_SHA1 authentication for identities that differ from the user running the DBusServer. Previously, a local attacker could manipulate symbolic links in their own home directory to bypass authentication and connect to a DBusServer with elevated privileges. The standard system and session dbus-daemons in their default configuration were immune to this attack because they did not allow DBUS_COOKIE_SHA1, but third-party users of DBusServer such as Upstart could be vulnerable. Thanks to Joe Vennix of Apple Information Security. (dbus#269, Simon McVittie) (bso ... build is running
SR#752324 -- could one have a review? Other than me!
(In reply to Dr. Werner Fink from comment #8) > SR#752324 -- could one have a review? Other than me! Sorry due to timezones I was done for Friday when you sent this, doing the update was on my today list if I hadn't heard a good reason to keep --enable-asserts. I this using --disable-asserts would be better then adding another patch, its also now how debian handle it. https://salsa.debian.org/utopia-team/dbus/blob/debian/master/debian/rules
(In reply to Simon Lees from comment #9) > (In reply to Dr. Werner Fink from comment #8) > > SR#752324 -- could one have a review? Other than me! > > Sorry due to timezones I was done for Friday when you sent this, doing the > update was on my today list if I hadn't heard a good reason to keep > --enable-asserts. > > I this using --disable-asserts would be better then adding another patch, > its also now how debian handle it. > https://salsa.debian.org/utopia-team/dbus/blob/debian/master/debian/rules The udeb_configure_flags rule with --disable-asserts is used for the udeb packages and those type of packages are only for internal build system for debian packages. The patch is also from debian (not utopia!) and is really simple: do not debug within a running system (no exit(3) nor abort(3) in system libraries): --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -185,7 +185,7 @@ const char *_dbus_no_memory_message = "Not enough memory"; static dbus_bool_t warn_initted = FALSE; static dbus_bool_t fatal_warnings = FALSE; -static dbus_bool_t fatal_warnings_on_check_failed = TRUE; +static dbus_bool_t fatal_warnings_on_check_failed = FALSE; static void init_warnings(void) could you please explain what is wrong with this approach? If you like to debug you are able to set the environment variable DBUS_FATAL_WARNINGS to 1. Currently we do not set DBUS_FATAL_WARNINGS at all, that is with the current default all customers and users out there do debugging their systems and do not know about nor can handle this. This causes a lot of trouble and bugzilla entries.
(In reply to Dr. Werner Fink from comment #10) > (In reply to Simon Lees from comment #9) > > (In reply to Dr. Werner Fink from comment #8) > > > SR#752324 -- could one have a review? Other than me! > > > > Sorry due to timezones I was done for Friday when you sent this, doing the > > update was on my today list if I hadn't heard a good reason to keep > > --enable-asserts. > > > > I this using --disable-asserts would be better then adding another patch, > > its also now how debian handle it. > > https://salsa.debian.org/utopia-team/dbus/blob/debian/master/debian/rules > > The udeb_configure_flags rule with --disable-asserts is used for the udeb > packages and those type of packages are only for internal build system for > debian packages. > > The patch is also from debian (not utopia!) Utopia is the team maintaining dbus within debian according to there documentation, It can also be confirmed by the fact that almost all the commits there are from debians dbus maintainer @smcv > > --- a/dbus/dbus-internals.c > +++ b/dbus/dbus-internals.c > @@ -185,7 +185,7 @@ const char *_dbus_no_memory_message = "Not enough > memory"; > > static dbus_bool_t warn_initted = FALSE; > static dbus_bool_t fatal_warnings = FALSE; > -static dbus_bool_t fatal_warnings_on_check_failed = TRUE; > +static dbus_bool_t fatal_warnings_on_check_failed = FALSE; > > static void > init_warnings(void) > > could you please explain what is wrong with this approach? If you like to > debug you are able to set the environment variable DBUS_FATAL_WARNINGS to 1. > Currently we do not set DBUS_FATAL_WARNINGS at all, that is with the current > default all customers and users out there do debugging their systems and do > not know about nor can handle this. This causes a lot of trouble and > bugzilla entries. Debian dropped this patch at the start of last year (see below), Red Hat doesn't carry it (atleast centos) which makes me nervous being the only distro using and testing the patch, it means that no one is really looking at what is going on after the abort meaning there could end up being any form of data corruption or security issues hiding after in failure code paths that no one is testing. I wouldn't really be comfortable without going through and having a good look at whats happening after. Currently I don't have time for that. * d/p/debian/Don-t-abort-on-fatal-warnings-by-default.patch: Remove patch. This was committed not long after the addition of the fatal-by-default _dbus_warn_check_failed() checks for programming errors, with the changelog message "This will be set to upstream default again at some point so if you have an application that prints a DBus warning get it fixed".
Then you might do the update to 1.12.16 to get the security issue CVE-2019-12749 fixed for Tumbleweed/Factory as well accordingly to the policy Factory First.
Beside this fedorea seems to switch to dbus-broker which seems IMHO a better written part of code in comparision to dbus-1
(In reply to Dr. Werner Fink from comment #12) > Then you might do the update to 1.12.16 to get the security issue > CVE-2019-12749 fixed for Tumbleweed/Factory as well accordingly to the > policy Factory First. Yep that was my plan
(In reply to Dr. Werner Fink from comment #13) > Beside this fedorea seems to switch to dbus-broker which seems IMHO a better > written part of code in comparision to dbus-1 This is also on my todo list in the medium term, likely before SLE-16 but I have some higher priority issues to fix in some of my other packages before I get to it.
Moving away from critical