summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Zeuthen <davidz@redhat.com>2007-05-24 15:04:08 -0400
committerDavid Zeuthen <davidz@redhat.com>2007-05-24 15:04:08 -0400
commit7cb6e7a62a1c2f7475bf24c48dce17892aa29715 (patch)
tree1f25323c1d58051e2ad434a7779c8b7fd8eba87d
parentd6b69ff7e8f8510a833e71401b18c949c1f51074 (diff)
create lock file with predictable permissionshal-0_5_8-branch
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.
-rw-r--r--tools/hal-storage-cleanup-all-mountpoints.c7
-rw-r--r--tools/hal-storage-shared.c2
2 files changed, 3 insertions, 6 deletions
diff --git a/tools/hal-storage-cleanup-all-mountpoints.c b/tools/hal-storage-cleanup-all-mountpoints.c
index aa8d6574..a997190e 100644
--- 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;
}
diff --git a/tools/hal-storage-shared.c b/tools/hal-storage-shared.c
index 15ce3b5e..ce582c25 100644
--- 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;