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]

Fixing a security hole in mount table.


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]