Bug#984590: Patch update for #984590
On Fri, Mar 05, 2021 at 02:51:47PM +, Mike Gabriel wrote: > the correct fix to scanlogd.init is this: > +chown scanlogd:nogroup $RDIR > +chown root:root $RDIR/empty No, this is still incorrect, as I explained in another message (I realize you sent this one before you could see mine). Alexander
Bug#984590: scanlogd: /run/scanlogd/empty must not be writeable by scanlogd process
Hi Mike, Thank you for creating the bug. On Fri, Mar 05, 2021 at 02:32:54PM +, Mike Gabriel wrote: > if [ ! -d $RDIR/empty ]; then > mkdir -p $RDIR/empty > -chown -R scanlogd:nogroup $RDIR > +chown scanlogd:nogroup $RDIR > +chown root:root $RDIR/empty > fi > @Alexander: you ok with this change? It should be sufficient, shouldn't it? No, and no. Two issues here: 1. ALL directories in the path must not be writable by any user other than root. This means all must have root as their owner. 2. If someone had already installed the previous package revision, these new chown's wouldn't be reached (because of the "if") and thus insecure permissions would persist over a package upgrade and service restart. I think we need this: mkdir -p $RDIR/empty # The directory and its parent directories must not be writable by anyone but root chown root:root $RDIR $RDIR/empty and drop the "if". "mkdir -p" is happy to ignore an already existing directory (this is part of what the "-p" option does). So just these 3 lines (including the comment) instead of the entire if/fi block. Alexander
Bug#831356: Option "non-unix" is broken and leads to segmentation fault
On Thu, Jul 14, 2016 at 04:02:18PM -0400, Jim Paris wrote: > The passwdqc module fails with a segmentation fault. This is because, > in non-unix mode, pam_sm_chauthtok builds up a fake "struct passwd" on > the stack: This bug is now fixed in upstream passwdqc 1.3.1, as announced here: http://www.openwall.com/lists/announce/2016/07/22/1 Jim, thanks again for reporting this bug. Jackson, please update the Debian package. Alexander
Bug#831356: Option "non-unix" is broken and leads to segmentation fault
Hi Jim, On Thu, Jul 14, 2016 at 04:02:18PM -0400, Jim Paris wrote: > With a pam configuration like: > > password required pam_passwdqc.so min=disabled,8,8,7,7 retry=1 non-unix > random=32 enforce=users > > The passwdqc module fails with a segmentation fault. This is because, > in non-unix mode, pam_sm_chauthtok builds up a fake "struct passwd" on > the stack: Thank you for reporting this bug. I can confirm it from looking at the source code, although the segfault does not happen for me - I guess a valid pointer just happens to be in that stack location on my system. I intend to fix it shortly and put out a new upstream passwdqc release, which Debian would then need to update to. Alexander
Bug#639151: [oss-security] [Pkg-xfce-devel] Bug#639151: Bug#639151: Bug#639151: Local privilege escalation
Hi,
I haven't been watching this discussion closely, but here are some
comments that might be of help:
On Fri, Aug 26, 2011 at 11:07:20AM +0200, Yves-Alexis Perez wrote:
Would something like:
diff --git a/src/dmrc.c b/src/dmrc.c
index bff1da8..9f38faf 100644
--- a/src/dmrc.c
+++ b/src/dmrc.c
@@ -80,11 +80,25 @@ dmrc_save (GKeyFile *dmrc_file, const gchar *username)
/* Update the users .dmrc */
if (user)
{
+ /* write the file as the user itself */
+ pid_t pid;
+ pid = fork();
+
+ if (pid == 0)
+ {
+if (setuid (user_get_uid(user)) 0)
+{
+ g_warning(Error changing uid for %s: %s, username,
g_strerror(errno));
+ _exit(EXIT_FAILURE);
+}
You also need to switch gid and groups, and you do not have to fork() if
you only switch euid/egid/groups or fsuid/fsgid/groups. The latter may
be less portable, but at least on Linux it affects only the current
thread in a multi-threaded process. Probably this difference is
irrelevant in your case, though.
Here's an example:
http://git.altlinux.org/people/ldv/packages/?p=pam.git;a=commitdiff;h=pam_modutil_priv
A tricky part is what to do when you have partially switched credentials
and one of the syscalls fails (e.g., you've switched the gid but not yet
the uid). The code referenced above (Linux-PAM commit) tries to restore
the old credentials, but ignores possible failure to do so. A better
action might be to terminate the current process on failure to restore
old credentials.
path = g_build_filename (user_get_home_directory (user), .dmrc,
NULL);
g_file_set_contents (path, data, length, NULL);
-if (getuid () == 0 chown (path, user_get_uid (user), user_get_gid
(user)) 0)
-g_warning (Error setting ownership on %s: %s, path, strerror
(errno));
g_free (path);
+_exit(EXIT_SUCCESS);
+
+ }
+ if (pid 0)
+wait(NULL);
}
You're lucky that you don't seem to need to pass the result of
g_file_set_contents() back to the parent process. If you were reading
rather than writing a file, you'd have difficulty using the fork()
approach. However, in your case fork() may actually be fine (but you do
need to drop gid and groups as well).
I hope this helps.
Alexander
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#543451: needlessly executable stack
Hi Kees, On Mon, Aug 24, 2009 at 07:04:01PM -0700, Kees Cook wrote: It seems that john is built (in some situation) against assembly code that lack stack markings[1]. This results in the entire program being built with an executable stack. The attached patch solve this by adding a default ASFLAGS option to turn off executable stacks when assembling. Yes, I am aware of this issue - for some years now, in fact. I did not fix it yet because I was worried that the proposed fixes would break portability to some older and/or non-Linux systems, and I did not have time to check (had more important stuff to do). Well, I checked the .section approach as used by Gentoo on an 11 years old Linux system just recently - and it worked (in the sense that it did not break the compile). So I think I will just use it with a proper #ifdef. As to the ASFLAGS change, it does break things on this same ancient system: gcc -c -Wa,--noexecstack x86.S /usr/i486-linux/bin/as: unrecognized option `--noexecstack' GNU assembler version 980303 (i586-linux), using BFD version 2.8.1.0.23 Meanwhile, it is up to you to choose any of these approaches for the Debian and Ubuntu packages. On a related note, I think that exec-shield lacks an enforcing mode (sysctl'able) where it would ignore those flags, because most binaries that it treats as potentially requiring executable stack actually don't. Thanks, Alexander -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#421085: FTBFS: scanlogd.c:181: error: 'CLK_TCK' undeclared
Debian fixed this bug incorrectly. CLOCKS_PER_SEC is not a correct substitute for CLK_TCK. Please see: http://www.openwall.com/lists/xvendor/2006/04/17/1 To make matters worse, the glibc documentation is buggy: http://sources.redhat.com/bugzilla/show_bug.cgi?id=2636 For scanlogd, the correct fix is to update to 2.2.6 - which, by the way, was already out by the time Debian wrongly fixed the bug. I find it likely that many other Debian packages have introduced similar bugs, though (blindly replacing CLK_TCK with CLOCKS_PER_SEC). Alexander -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#496375: The possibility of attack with the help of symlinks in some Debian packages
On Wed, Aug 27, 2008 at 09:06:58AM +0200, Julien Valroff wrote: Do you suggest that using /var/run/rkhunter-debug is better than /tmp/rkhunter-debug. (created using mktemp)? Yes - primarily from usability standpoint. This time, having a fixed filename is better, and since rkhunter needs to be run as root anyway (does it?), /var/run should do and be safe. However, if I am wrong in my assumption that rkhunter requires root, then indeed /var/run is not appropriate - and the mktemp approach makes sense. or is that still using mktemp to create a /var/run/rkhunter-debug.XX file? No. Can you explain why it is more secure? That was not the point I was making. Rather, the point was/is that mktemp is normally used for program-internal and truly temporary files, and this time we have a file that is meant to be accessed by a human user - so a fixed filename in a directory only writable by root may be more appropriate. However, once again, if rkhunter may reasonably be run by non-root (I just don't know, I've never used rkhunter), then mktemp -t ... may be appropriate as it will retain that capability. Alexander -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#496375: The possibility of attack with the help of symlinks in some Debian packages
FWIW, I happened to independently notice this and report it upstream a week ago: https://sourceforge.net/tracker/?func=detailatid=794190aid=1971965group_id=155034 While I am at it, I suggest that you change /tmp/rkhunter-debug to /var/run/rkhunter-debug. Right now, you have a security hole allowing for local root compromise, although indeed the race condition is hard to trigger in practice. To those reading this: please note that this suggestion by no means constitutes a security review of rkhunter by me. I notice that the Debian package was fixed to use mktemp; I think that a fixed filename under /var/run would be better in this case. Also, rkhunter could be patched to enforce mode 600 on the file, regardless of umask. (mktemp does that, but when a fixed filename under /var/run is used instead, that would need to be explicit.) Oh, and I was probably wrong about the race condition being hard to trigger - I forgot about directory notifications for a moment. Also, when using mktemp it is important to check for possible failure of mktemp - e.g., with || exit on the line (which propagates mktemp's exit code to that of the script). On Openwall GNU/*/Linux (Owl), we use the following shell script snippets for real temporary files (which are meant to be gone when the script terminates), as documented in Owl/doc/CONVENTIONS - | It's better to not use temporary files, however if you must, the | preferred way to do it from shell scripts is with code like this: | | TMPFILE=`mktemp -t program.XX` || exit | trap 'rm -f -- $TMPFILE' EXIT | trap 'trap - EXIT; rm -f -- $TMPFILE; exit 1' HUP INT QUIT TERM | | To create temporary directories, use: | | TMPD=`mktemp -dt program.XX` || exit | trap 'rm -rf -- $TMPD' EXIT | trap 'trap - EXIT; rm -rf -- $TMPD; exit 1' HUP INT QUIT TERM Alexander -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#403798: this bug/#403798: john: please don't clobber ./restore
Disclaimer: I am not a Debian user, I merely wrote JtR. My understanding is that some supported versions of Debian continue to use the ancient JtR 1.6, released over 8 years ago. I recommend that you start by updating to JtR 1.7+, which includes system-wide installation support. IIRC, this was already done in testing. JtR 1.6 was not meant to be packaged at all, which is why doing so results in some weirdness. My recommendation is that you use the latest 1.7.x for testing and unstable and use the latest 1.7.0.x for stable. On Mon, Jan 01, 2007 at 08:34:54PM -0500, Justin Pryzby wrote: Alex, please feel free to comment. Ideally, Debian would apply the same patch used upstream (but perhaps before you make a new release). Is the issue that a file that just happens to be called restore but is unrelated to JtR may be overwritten? If so, this is resolved by upgrading to JtR 1.7 with system-wide installation support enabled. Thank you for the opportunity to comment on this! Alexander -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
