From: David Zeuthen <davidz@redhat.com>
Date: Thu, 24 May 2007 19:04:08 +0000 (-0400)
Subject: create lock file with predictable permissions
X-Git-Tag: HAL_0_2
X-Git-Url: http://gitweb.freedesktop.org/?p=hal.git;a=commitdiff;h=7cb6e7a62a1c2f7475bf24c48dce17892aa29715

create lock file with predictable permissions

This was initially reported as a RHEL5 bug #241032, but after analysis
it was determine there is no security issue

Steve Grubb <sgrubb@redhat.com>:
> I was working on an IDS script and found this file:
>
> ls -l  /media/.hal-mtab-lock
> --ws--Sr-x 1 root root 0 Nov 15  2006 /media/.hal-mtab-lock
>
> Looking at open 3p man page, if you pass the O_CREAT flag to open, it
> will look for arg 3 to get the mode. In this case it turns out to be
> the stack contents. I wonder how dangerous this could be if by random
> chance its setuid root and world writable?

David Zeuthen <davidz@redhat.com>:
> Steve, thanks for catching that. Using mode 0600 is the right
> approach; the lock file is private to HAL components all running as
> uid 0. Also, on HAL startup we need to delete the lock file otherwise
> it won't get recreated with the appropriate mode.

David Zeuthen <davidz@redhat.com>:
> Hmm, playing around with passing various modes in; when I pass 04777 I
> get this
>
> $ ls -l /media/.hal-mtab-lock
> -rwsr-xr-x 1 root root 0 2007-05-23 17:31 /media/.hal-mtab-lock
>
> which makes sense as hald calls umask(022) when it daemonizes and this is
> inherited by the process that creates the lock file. Hence, we will never ever
> write files in this situation that are world writable. Since the the lock file
> is always zero bytes we're good. There is no risk of attack here.
---

--- a/tools/hal-storage-cleanup-all-mountpoints.c
+++ b/tools/hal-storage-cleanup-all-mountpoints.c
@@ -162,9 +162,8 @@ do_cleanup (void)
 int
 main (int argc, char *argv[])
 {
-	if (!lock_hal_mtab ()) {
-		unknown_error ("Cannot obtain lock on /media/.hal-mtab");
-	}
+
+        unlink ("/media/.hal-mtab-lock");
 
 	if (getenv ("HAL_PROP_INFO_UDI") == NULL)
 		usage ();
@@ -174,7 +173,5 @@ main (int argc, char *argv[])
 #endif
 	do_cleanup ();
 
-
-	unlock_hal_mtab ();
 	return 0;
 }
--- a/tools/hal-storage-shared.c
+++ b/tools/hal-storage-shared.c
@@ -560,7 +560,7 @@ lock_hal_mtab (void)
 
 	printf ("%d: XYA attempting to get lock on /media/.hal-mtab-lock\n", getpid ());
 
-	lock_mtab_fd = open ("/media/.hal-mtab-lock", O_CREAT | O_RDWR);
+	lock_mtab_fd = open ("/media/.hal-mtab-lock", O_CREAT | O_RDWR, 0600);
 
 	if (lock_mtab_fd < 0)
 		return FALSE;