pam_ecryptfs doesn't drop gid when using user's files

Bug #732614 reported by Vasily Kulikov
270
This bug affects 2 people
Affects Status Importance Assigned to Milestone
eCryptfs
Fix Released
High
Tyler Hicks
ecryptfs-utils (Ubuntu)
Fix Released
High
Tyler Hicks

Bug Description

I report several bug as a single ticket as they have approximately the same significance and it's more simple to fix them at a time.

1) pam_ecryptfs heavily use non-root files, dropping EUID=0, but forgets to drop EGID=0. To exploit it try to create symlink ~/.ecryptfs/auto-mount to some existing (not not visible to nonroot) root owned file, run inotifywait on ~/.ecryptfs/ and try to login as a user, triggering pam_ecryptfs code. Then compare with nonexisting root file.

Some filesystem syscalls are used with EUID=0: inside of ecryptfs_pam_automount_set(), ecryptfs_fetch_private_mnt(), ecryptfs_read_salt_hex_from_rc().

2) It runs (u)mount.ecryptfs_private with GID=0. mount.ecryptfs_private drops only UID, so it becomes vulnerable to leakage of existence of files with group=root, like (1).

3) All results of set*id() syscalls must be checked not to repeat sendmail bug. There is actually one real bug with not checking return code: wrap_passphrase_if_necessary() calls setuid() (not seteuid()!) and doesn't restore full root.

4) generate_nv_list() calls strlen() with argument to user controllable data. The data might be not terminating with \0. This may lead to SEGFAULT.

5) src/libecryptfs/key_management.c calls read(), but doesn't check how many bytes were read. This might lead to a leakage of contents of stack memory of privileged process.

Note that (2) may not be fixed by dropping gid=0 with setresgid() as the process would become ptrace'able. As it still owns root-owned tty, etc., this would lead to privilege escalation.

CVE References

Revision history for this message
Dustin Kirkland  (kirkland) wrote :

Thanks for the report. Will get these fixed.

Changed in ecryptfs:
status: New → Triaged
importance: Undecided → High
assignee: nobody → Dustin Kirkland (kirkland)
Changed in ecryptfs-utils (Ubuntu):
status: New → Triaged
importance: Undecided → High
assignee: nobody → Dustin Kirkland (kirkland)
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Apologies for this report slipping through the cracks. It is next up on my todo list and I plan to start work on it by the end of the week.

Changed in ecryptfs:
assignee: Dustin Kirkland (kirkland) → Tyler Hicks (tyhicks)
Changed in ecryptfs-utils (Ubuntu):
assignee: Dustin Kirkland (kirkland) → Tyler Hicks (tyhicks)
Curtis Hovey (sinzui)
no longer affects: ecryptfs-utils (Fedora)
no longer affects: ecryptfs-utils (Debian)
Tyler Hicks (tyhicks)
Changed in ecryptfs:
status: Triaged → In Progress
Revision history for this message
Tyler Hicks (tyhicks) wrote :

I had began working on the fixes but then set them aside and never completed them.

The good news is that Sebastian Krahmer submitted a patch which fixed nearly all of these issues (plus some others) in a more complete manner than what I had written. It has been committed to trunk as revno 705. I fixed (5) in revno 706.

(4) is not fixed by either of these revisions. It does not sound to me like it has security implications, but please let me know if I'm missing something. I'll open a new, non-security bug for it.

Changed in ecryptfs:
status: In Progress → Fix Committed
Revision history for this message
Tyler Hicks (tyhicks) wrote :

(4) was reopened as bug #1023323

I'm making this bug public now since fixes are committed upstream.

visibility: private → public
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ecryptfs-utils - 99-0ubuntu1

---------------
ecryptfs-utils (99-0ubuntu1) quantal; urgency=low

  [ Dustin Kirkland ]
  * debian/ecryptfs-utils.postinst: LP: #936093
    - ensure desktop file is executable
  * precise

  [ Wesley Wiedenmeier ]
  * src/utils/mount.ecryptfs.c: LP: #329264
    - remove old hack, that worked around a temporary kernel regression;
      ensure that all mount memory is mlocked

  [ Sebastian Krahmer ]
  * src/pam_ecryptfs/pam_ecryptfs.c: LP: #732614
    - drop group privileges in the same places that user privileges are
      dropped
    - check return status of setresuid() calls and return if they fail
    - drop privileges before checking for the existence of
      ~/.ecryptfs/auto-mount to prevent possible file existence leakage
      by a symlink to a path that typically would not be searchable by
      the user
    - drop privileges before reading salt from the rc file to prevent the
      leakage of root's salt and, more importantly, using the incorrect salt
    - discovered, independently, by Vasiliy Kulikov and Sebastian Krahmer
  * src/pam_ecryptfs/pam_ecryptfs.c: LP: #1020904
    - after dropping privileges, clear the environment before executing the
      private eCryptfs mount helper
    - discovered by Sebastian Krahmer
  * src/utils/mount.ecryptfs_private.c: LP: #1020904
    - do not allow private eCryptfs mount aliases to contain ".." characters
      as a preventative measure against a crafted file path being used as an
      alias
    - force the MS_NOSUID mount flag to protect against user controlled lower
      filesystems, such as an auto mounted USB drive, that may contain a
      setuid-root binary
      + CVE-2012-3409
    - force the MS_NODEV mount flag
    - after dropping privileges, clear the environment before executing umount
    - discovered by Sebastian Krahmer

  [ Tyler Hicks ]
  * src/libecryptfs/key_management.c: LP: #732614
    - zero statically declared buffers to prevent the leakage of stack
      contents in the case of a short file read
    - discovered by Vasiliy Kulikov
  * src/libecryptfs/module_mgr.c, src/pam_ecryptfs/pam_ecryptfs.c:
    - fix compiler warnings
 -- Dustin Kirkland <email address hidden> Fri, 13 Jul 2012 09:52:36 -0500

Changed in ecryptfs-utils (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Released in ecryptfs-utils-99

Changed in ecryptfs:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.