diff --git a/ChangeLog b/ChangeLog index 7172fb117..8af84063e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2004-03-27 Richard Frith-Macdonald + + * NSMessagePort.m: + * NSMessagePortNameServer.m: Use base library methods for file + management rather than unix syscalls. + +2004-03-27 Alexander Malmberg + + * NSUser.m: Plug some security loopholes in temporary directory + 2004-03-27 Mark Allison * Headers/Additions/GNUstepBase/GSXML.h: diff --git a/Source/NSMessagePort.m b/Source/NSMessagePort.m index b6cc389fa..9a6b73ddd 100644 --- a/Source/NSMessagePort.m +++ b/Source/NSMessagePort.m @@ -39,6 +39,9 @@ #include "Foundation/NSConnection.h" #include "Foundation/NSDebug.h" #include "Foundation/NSPathUtilities.h" +#include "Foundation/NSValue.h" +#include "Foundation/NSFileManager.h" +#include "Foundation/NSProcessInfo.h" #include #include #ifdef HAVE_UNISTD_H @@ -1186,19 +1189,27 @@ static unsigned wordAlign; + (id) new { static int unique_index = 0; - NSString *path; + NSString *path; + NSNumber *p = [NSNumber numberWithInt: 0700]; + NSDictionary *attr; + + attr = [NSDictionary dictionaryWithObject: p + forKey: NSFilePosixPermissions]; path = NSTemporaryDirectory(); path = [path stringByAppendingPathComponent: @"NSMessagePort"]; - mkdir([path fileSystemRepresentation], 0700); + [[NSFileManager defaultManager] createDirectoryAtPath: path + attributes: attr]; path = [path stringByAppendingPathComponent: @"ports"]; - mkdir([path fileSystemRepresentation], 0700); + [[NSFileManager defaultManager] createDirectoryAtPath: path + attributes: attr]; M_LOCK(messagePortLock); path = [path stringByAppendingPathComponent: - [NSString stringWithFormat: @"%i.%i", getpid(), unique_index++]]; + [NSString stringWithFormat: @"%i.%i", + [[NSProcessInfo processInfo] processIdentifier], unique_index++]]; M_UNLOCK(messagePortLock); return RETAIN([self _portWithName: [path fileSystemRepresentation] diff --git a/Source/NSMessagePortNameServer.m b/Source/NSMessagePortNameServer.m index 3c34e7483..7fd8bed69 100644 --- a/Source/NSMessagePortNameServer.m +++ b/Source/NSMessagePortNameServer.m @@ -7,6 +7,8 @@ #include "Foundation/NSMapTable.h" #include "Foundation/NSPathUtilities.h" #include "Foundation/NSPort.h" +#include "Foundation/NSFileManager.h" +#include "Foundation/NSValue.h" #include #include @@ -104,13 +106,20 @@ static void clean_up_names(void) [serverLock lock]; if (!base_path) { + NSNumber *p = [NSNumber numberWithInt: 0700]; + NSDictionary *attr; + path = NSTemporaryDirectory(); + attr = [NSDictionary dictionaryWithObject: p + forKey: NSFilePosixPermissions]; path = [path stringByAppendingPathComponent: @"NSMessagePort"]; - mkdir([path fileSystemRepresentation], 0700); + [[NSFileManager defaultManager] createDirectoryAtPath: path + attributes: attr]; path = [path stringByAppendingPathComponent: @"names"]; - mkdir([path fileSystemRepresentation], 0700); + [[NSFileManager defaultManager] createDirectoryAtPath: path + attributes: attr]; base_path = RETAIN(path); } diff --git a/Source/NSUser.m b/Source/NSUser.m index 2ebf8d15d..df8f652c3 100644 --- a/Source/NSUser.m +++ b/Source/NSUser.m @@ -774,6 +774,9 @@ NSStandardLibraryPaths(void) /** * Returns the name of a directory in which temporary files can be stored. * Under GNUstep this is a location which is not readable by other users. + *
+ * If a suitable directory can't be found or created, this function raises an + * NSGenericException. */ NSString * NSTemporaryDirectory(void) @@ -783,6 +786,8 @@ NSTemporaryDirectory(void) NSString *baseTempDirName = nil; NSDictionary *attr; int perm; + int owner; + int uid; BOOL flag; #if defined(__WIN32__) char buffer[1024]; @@ -830,19 +835,32 @@ NSTemporaryDirectory(void) if ([manager fileExistsAtPath: tempDirName isDirectory: &flag] == NO || flag == NO) { - NSLog(@"Temporary directory (%@) does not seem to exist", tempDirName); - return nil; + [NSException raise: NSGenericException + format: @"Temporary directory (%@) does not exist", + tempDirName]; + return nil; /* Not reached. */ } /* - * Check that the directory owner (presumably us) has access to it, - * and nobody else. If other people have access, try to create a - * secure subdirectory. + * Check that we are the directory owner, and that we, and nobody else, + * have access to it. If other people have access, try to create a secure + * subdirectory. */ attr = [manager fileAttributesAtPath: tempDirName traverseLink: YES]; + owner = [[attr objectForKey: NSFileOwnerAccountID] intValue]; perm = [[attr objectForKey: NSFilePosixPermissions] intValue]; perm = perm & 0777; - if (perm != 0700 && perm != 0600) + +#if defined(__MINGW__) + uid = owner; +#else +#ifdef HAVE_GETEUID + uid = geteuid(); +#else + uid = getuid(); +#endif /* HAVE_GETEUID */ +#endif + if ((perm != 0700 && perm != 0600) || owner != uid) { /* NSLog(@"Temporary directory (%@) may be insecure ... attempting to " @@ -860,16 +878,35 @@ NSTemporaryDirectory(void) if ([manager createDirectoryAtPath: tempDirName attributes: attr] == NO) { - tempDirName = baseTempDirName; - NSLog(@"Temporary directory (%@) may be insecure", tempDirName); + [NSException raise: NSGenericException + format: @"Attempt to create a secure temporary directory (%@) failed.", + tempDirName]; + return nil; /* Not reached. */ } } + + /* + * Check that the new directory is really secure. + */ + attr = [manager fileAttributesAtPath: tempDirName traverseLink: YES]; + owner = [[attr objectForKey: NSFileOwnerAccountID] intValue]; + perm = [[attr objectForKey: NSFilePosixPermissions] intValue]; + perm = perm & 0777; + if ((perm != 0700 && perm != 0600) || owner != uid) + { + [NSException raise: NSGenericException + format: @"Attempt to create a secure temporary directory (%@) failed.", + tempDirName]; + return nil; /* Not reached. */ + } } if ([manager isWritableFileAtPath: tempDirName] == NO) { - NSLog(@"Temporary directory (%@) is not writable", tempDirName); - return nil; + [NSException raise: NSGenericException + format: @"Temporary directory (%@) is not writable", + tempDirName]; + return nil; /* Not reached. */ } return tempDirName; }