[PATCH] gprocess: Fix --no-caps handling when building with hardening options.
When both glib and syslog-ng is built with hardening options enabled, overwriting a string pointer with a FALSE boolean does not have the expected results: the string will not become NULL. In this case, --no-caps will end up with a segfault, because process_opts.caps is non-NULL, and points to junk. To fix this, introduce a new function that sets process_opts.caps to NULL explicitly, and use this function as a callback, instead of assuming that a FALSE boolean will have the expected results. Reported-By: Peter Czanik <czanik@balabit.hu> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/gprocess.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/lib/gprocess.c b/lib/gprocess.c index 2ccaffe..aaefbab 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -1440,6 +1440,14 @@ g_process_process_mode_arg(const gchar *option_name G_GNUC_UNUSED, const gchar * return TRUE; } +static gboolean +g_process_process_no_caps(const gchar *option_name G_GNUC_UNUSED, const gchar *value G_GNUC_UNUSED, + gpointer data G_GNUC_UNUSED, GError *error) +{ + process_opts.caps = NULL; + return TRUE; +} + static GOptionEntry g_process_option_entries[] = { { "foreground", 'F', G_OPTION_FLAG_REVERSE, G_OPTION_ARG_NONE, &process_opts.mode, "Do not go into the background after initialization", NULL }, @@ -1450,7 +1458,7 @@ static GOptionEntry g_process_option_entries[] = { "gid", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &process_opts.group, NULL, NULL }, { "chroot", 'C', 0, G_OPTION_ARG_STRING, &process_opts.chroot_dir, "Chroot to this directory", "<dir>" }, { "caps", 0, 0, G_OPTION_ARG_STRING, &process_opts.caps, "Set default capability set", "<capspec>" }, - { "no-caps", 0, G_OPTION_FLAG_REVERSE, G_OPTION_ARG_NONE, &process_opts.caps, "Disable managing Linux capabilities", NULL }, + { "no-caps", 0, G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, g_process_process_no_caps, "Disable managing Linux capabilities", NULL }, { "pidfile", 'p', 0, G_OPTION_ARG_STRING, &process_opts.pidfile, "Set path to pid file", "<pidfile>" }, { "enable-core", 0, 0, G_OPTION_ARG_NONE, &process_opts.core, "Enable dumping core files", NULL }, { "fd-limit", 0, 0, G_OPTION_ARG_INT, &process_opts.fd_limit_min, "The minimum required number of fds", NULL }, -- 1.7.7.2
On Sat, 2011-11-12 at 14:27 +0100, Gergely Nagy wrote:
When both glib and syslog-ng is built with hardening options enabled, overwriting a string pointer with a FALSE boolean does not have the expected results: the string will not become NULL.
Hmm... what kind of hardening options are these? I haven't heard about them yet. FALSE is not a numeric zero? That'll probably break other assumptions in syslog-ng, not just this one. Can you point me in the right direction? Thanks. -- Bazsi
Balazs Scheidler <bazsi@balabit.hu> writes:
On Sat, 2011-11-12 at 14:27 +0100, Gergely Nagy wrote:
When both glib and syslog-ng is built with hardening options enabled, overwriting a string pointer with a FALSE boolean does not have the expected results: the string will not become NULL.
Hmm... what kind of hardening options are these? I haven't heard about them yet.
FALSE is not a numeric zero? That'll probably break other assumptions in syslog-ng, not just this one.
It is a numeric zero. It probably has to do with trying to shovel an integer into a gpointer, and one of the hardening flags being to clever and doing something silly.
Can you point me in the right direction?
As far as I see, these are: CFLAGS: -fPIE -fstack-protector -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security LDFLAGS: -pie -Wl,-z,relro -Wl,-z,now I'm not exactly sure which one is the problem (I'm not 100% sold that this is the cause, either). But on Ubuntu Lucid, compiling a syslog-ng with -fstack-protector -D_FORTIFY_SOURCE=2 does seem to trigger the issue with --no-caps. However, doing the same on Debian sid does not. So it might be something in ubuntu's glib.. I wasn't able to get much further than that, unfortunately. -- |8]
On Sat, 2011-11-12 at 21:03 +0100, Gergely Nagy wrote:
Balazs Scheidler <bazsi@balabit.hu> writes:
On Sat, 2011-11-12 at 14:27 +0100, Gergely Nagy wrote:
When both glib and syslog-ng is built with hardening options enabled, overwriting a string pointer with a FALSE boolean does not have the expected results: the string will not become NULL.
Hmm... what kind of hardening options are these? I haven't heard about them yet.
FALSE is not a numeric zero? That'll probably break other assumptions in syslog-ng, not just this one.
It is a numeric zero. It probably has to do with trying to shovel an integer into a gpointer, and one of the hardening flags being to clever and doing something silly.
Can you point me in the right direction?
As far as I see, these are:
CFLAGS: -fPIE -fstack-protector -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security LDFLAGS: -pie -Wl,-z,relro -Wl,-z,now
I'm not exactly sure which one is the problem (I'm not 100% sold that this is the cause, either). But on Ubuntu Lucid, compiling a syslog-ng with -fstack-protector -D_FORTIFY_SOURCE=2 does seem to trigger the issue with --no-caps. However, doing the same on Debian sid does not. So it might be something in ubuntu's glib..
I wasn't able to get much further than that, unfortunately.
Ok, I thought that FALSE is not a numeric zero. Isn't this the problem? sizeof(gboolean) == 4 sizeof(gpointer) == 8 Isn't that the root cause? -- Bazsi
Ok, I thought that FALSE is not a numeric zero. Isn't this the problem?
sizeof(gboolean) == 4 sizeof(gpointer) == 8
Isn't that the root cause?
That might well be the problem, yeah. -- |8]
participants (3)
-
Balazs Scheidler
-
Gergely Nagy
-
Gergely Nagy