This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
This is the first in a series of patches fixing security holes associated with the file mappings in the core of Cygwin. I hope the explanations below are clear! Background on the mount table: System and user mounts are kept in a FileMapping, shared by all programs started from Windows by a given user, and their descendants, even after seteuid. User mounts are stored permanently under the HKCU registry. The user pointed to by HKCU depends on the user SID in the current thread access token. It changes during the seteuid call. So a seteuid'ed user creating a user mount will make an entry in the mount file mapping of its ancestor, and will also store it in its own user registry. That makes for a weird combination. For consistency the mount file mapping should be recreated following seteuid. The file mapping should be named after the SID (on NT), not the user name, to avoid name aliasing. To make sure HKCU is meaningful loaded, load_registry_hive should be called in seteuid, calling it in spawn is too late. The current code also creates a security gap that is easy to exploit: Telnet in as a non-privileged user. Create a $HOME/etc directory and cp -R /etc to it, give 777 access. Edit $HOME/etc/passwd and remove all the passwords. mount -u `cygpath -m "$HOME/etc"` /etc Bingo, all the programs running as SYSTEM will now switch to the new mapping for /etc. One can telnet in as any user without a password. (FWIW, here are two extra observations: - programs running as SYSTEM cannot umount -u /etc [because it's not under their HKCU], but they can mount -f -u over it. - programs running as SYSTEM use the user mounts of the .Default user, at least on NT4.) This simple attack would not work with the change described above, but the SYSTEM file mapping could still be manipulated by writing a program that directly accesses it. The solution is to specify appropriate security attributes when creating the file mapping. Using sec_none is fine, don't worry about the name. When initially testing the patch I found that the user mounts were set correctly from sshd but not from telnetd (with or without passwd). Telnetd (login) was producing the following error when reading HKCU: 255 1183303 [main] a 266 mount_table_initialize: initializing mount table 432 1183735 [main] a 266 reg_key::build_reg: failed to create key SOFTWARE in the registry 422 1184157 [main] a 266 reg_key::build_reg: failed to create key SOFTWARE in the registry 772 1184929 [main] a 266 mount_info::read_mounts: RegEnumKeyEx failed, error 6! 778 1185707 [main] a 266 mount_info::add_item: e:\[e:], /[/], 0xA <= XXX system mounts OK reg_key::build_reg does not print %E, but gdb reveals it is 5. I then found out that HKCU can behave weirdly <http://support.microsoft.com/default.aspx?scid=kb;en-us;199190> and implemented the short term workaround outlined therein. According to MS we should not use HKCU if several thread access it. The attached patch is the minimum that fixes the problem. A follow up patch, touching more files, will cleanup some details. Including it at this stage would be needlessly confusing. In summary this patch: 1) moves load_registry_hive from spawn_guts() to seteuid(). 2) moves all the mount table initialization from memory_init into a new function, user_shared_initialize [this choice of name will become clear later], which is then called from both memory_init and seteuid. 3) Adds an optional security attributes argument to open_shared. 4) fixes an unrelated (and currently non exposed) bug in security.h (sec_user). Pierre 2003-09-08 Pierre Humblet <pierre.humblet@ieee.org> * shared_info.h: Include security.h. (open_shared): Add psa argument. (user_shared_initialize): New declaration. * security.h: Add _SECURITY_H guard. (sec_user): Use sec_none in the no ntsec case. * spawn.cc (spawn_guts): Remove call to load_registry_hive. * syscalls (seteuid32): If warranted, call load_registry_hive, user_shared_initialize and RegCloseKey(HKEY_CURRENT_USER). * shared.cc (user_shared_initialize): New. (open_shared): Add and use psa argument. (memory_init): Move mount table initialization to user_shared_initialize. Call it.
Attachment:
shared.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |