Bug 141258

Summary: /etc/rc.d/tmp may act incorrectly based on unprivleged local user actions
Product: Base System Reporter: Jon Passki <jon>
Component: confAssignee: Jaakko Heinonen <jh>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Unspecified   
Hardware: Any   
OS: Any   

Description Jon Passki 2009-12-07 22:50:06 UTC
/etc/rc.d/tmp may remount /tmp or halt the boot process based on unprivleged local user actions.  The default behavior of tmpmfs is defined in /etc/defaults/rc.conf and set to AUTO. This causes the '*' case to be hit in the /etc/rc.d/tmp script. If a local user creates a file (not a directory) in /tmp called .diskless and the system is rebooted or the script is called directly, the system will either drop into /bin/sh prior to reaching DAEMON or the system will remount /tmp with a potentially smaller size than expected. Both of these conditions are probably not ideal and the prior condition could lead to a boot-up DoS, depending upon local system configurations. The latter condition is harder to fix once the system is in multi-user mode, and especially if users connect via SSH. This is because the /tmp directory will contain open files and/or sockets. So, a fix in this case would also require dropping the system into single-user mode.

Refer to http://blog.cykyc.org/2009/12/issue-with-freebsd-etcrcdtmp-script.html for a full write-up.

Fix: 

No fix at this time.

Recommendation:
My initial idea at a fix is to include a new variable in /etc/rc.d/cleartmp that would be set to YES in /etc/defaults/rc.conf. The variable would be similar to clear_tmp_X (maybe called clear_tmp_safe?), calling a routine to wipe and remake the directory /tmp/.diskless. Once this script was ran by root, a subsequent call to /etc/rc.d/tmp upon reboot or directory would act right for most cases.

Diskless clients should be OK, though, as long as /tmp/.diskless is not included in /etc/mtree/BSD.root.dist. When /etc/rc.d/cleartmp would run the first time, it would already be on a memory /tmp file system (assuming /conf doesn't contain anything to point to a residual mount point that could have been tampered). Clients also using memory-backed /tmp should be OK, since the variable will force a creation of a memory-backed /tmp mount point. But, some other eyes should look at this prior to changing the behavior.
How-To-Repeat: Sample attack on a symlink'd /tmp:

> ls -la /tmp
lrwxr-xr-x  1 root  wheel  7 Dec  7 15:55 /tmp -> var/tmp
> cd /tmp
> ll
total 12
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .ICE-unix
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .X11-unix
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .XIM-unix
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .font-unix
-rw-r--r--  1 root  wheel    0 Dec  7 15:55 foo
drwxrwxrwt  2 root  wheel  512 Dec  7 15:44 vi.recover
> ln foo .diskless
> ll
total 12
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .ICE-unix
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .X11-unix
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .XIM-unix
-rw-r--r--  2 root  wheel    0 Dec  7 15:55 .diskless
drwxrwxrwt  2 root  wheel  512 Dec  7 15:55 .font-unix
-rw-r--r--  2 root  wheel    0 Dec  7 15:55 foo
drwxrwxrwt  2 root  wheel  512 Dec  7 15:44 vi.recover
> su -
Password:
test-8# /etc/rc.d/tmp start
*** /tmp is a symlink to a non-writable area!
dropping into shell, ^D to continue anyway.
#


Sample attack on a mounted /tmp:
> mount
/dev/ad0s1a on / (ufs, local)
devfs on /dev (devfs, local, multilabel)
/dev/ad0s1f on /usr (ufs, local, soft-updates)
/dev/ad0s1d on /var (ufs, local, soft-updates)
/dev/ad0s1e on /tmp (ufs, local, soft-updates)
> df /tmp
Filesystem  1K-blocks Used  Avail Capacity  Mounted on
/dev/ad0s1e    507630   16 467004     0%    /tmp
> ll /tmp
total 14
drwxrwxrwt  2 root  wheel     512 Dec  7 15:22 .ICE-unix
drwxrwxrwt  2 root  wheel     512 Dec  7 15:22 .X11-unix
drwxrwxrwt  2 root  wheel     512 Dec  7 15:22 .XIM-unix
drwxrwxrwt  2 root  wheel     512 Dec  7 15:22 .font-unix
drwxrwxr-x  2 root  operator  512 Dec  7 15:10 .snap
-rw-r--r--  1 root  wheel       0 Dec  7 15:25 foo
> ln foo .diskless
> su -
Password:
test-8# /etc/rc.d/tmp start
test-8# mount
/dev/ad0s1a on / (ufs, local)
devfs on /dev (devfs, local, multilabel)
/dev/ad0s1f on /usr (ufs, local, soft-updates)
/dev/ad0s1d on /var (ufs, local, soft-updates)
/dev/ad0s1e on /tmp (ufs, local, soft-updates)
/dev/md0 on /tmp (ufs, local)
test-8# df /tmp
Filesystem 1K-blocks Used Avail Capacity  Mounted on
/dev/md0       19566    4 17998     0%    /tmp
test-8# ll /tmp
total 2
drwxrwxr-x  2 root  operator  512 Dec  7 15:59 .snap
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2009-12-08 03:01:22 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-rc

Over to maintainer(s).
Comment 2 Jaakko Heinonen freebsd_committer freebsd_triage 2010-03-05 06:15:40 UTC
Here is a proposed minimal fix for the problem. mktemp(1) should be
available because the script requires mountcritremote. (Can someone
confirm this?)

%%%
Index: etc/rc.d/tmp
===================================================================
--- etc/rc.d/tmp	(revision 204194)
+++ etc/rc.d/tmp	(working copy)
@@ -51,8 +51,8 @@ case "${tmpmfs}" in
 [Nn][Oo])
 	;;
 *)
-	if /bin/mkdir -p /tmp/.diskless 2> /dev/null; then
-		rmdir /tmp/.diskless
+	if _tmpdir=$(mktemp -d -q /tmp/.diskless.XXXXXX); then
+		rmdir ${_tmpdir}
 	else
 		if [ -h /tmp ]; then
 			echo "*** /tmp is a symlink to a non-writable area!"
%%%
Comment 3 Jon Passki 2010-03-05 12:29:06 UTC
On Fri, Mar 5, 2010 at 12:15 AM, Jaakko Heinonen <jh@freebsd.org> wrote:
>
> Here is a proposed minimal fix for the problem. mktemp(1) should be
> available because the script requires mountcritremote. (Can someone
> confirm this?)
>
> %%%
> Index: etc/rc.d/tmp
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- etc/rc.d/tmp =A0 =A0 =A0 =A0(revision 204194)
> +++ etc/rc.d/tmp =A0 =A0 =A0 =A0(working copy)
> @@ -51,8 +51,8 @@ case "${tmpmfs}" in
> =A0[Nn][Oo])
> =A0 =A0 =A0 =A0;;
> =A0*)
> - =A0 =A0 =A0 if /bin/mkdir -p /tmp/.diskless 2> /dev/null; then
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 rmdir /tmp/.diskless
> + =A0 =A0 =A0 if _tmpdir=3D$(mktemp -d -q /tmp/.diskless.XXXXXX); then
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 rmdir ${_tmpdir}
> =A0 =A0 =A0 =A0else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if [ -h /tmp ]; then
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0echo "*** /tmp is a symlin=
k to a non-writable area!"
> %%%
>

Seems like an appropriate fix. I don't have a way to test this out at
the moment though.

Jon
Comment 4 Jaakko Heinonen freebsd_committer freebsd_triage 2010-03-12 06:35:13 UTC
Responsible Changed
From-To: freebsd-rc->jh

Take.
Comment 5 dfilter service freebsd_committer freebsd_triage 2010-03-13 12:02:59 UTC
Author: jh
Date: Sat Mar 13 12:02:44 2010
New Revision: 205121
URL: http://svn.freebsd.org/changeset/base/205121

Log:
  Use an unique directory name instead of hardcoded /tmp/.diskless.
  A malicious user could create a file named /tmp/.diskless and cause
  the script to misbehave.
  
  PR:		conf/141258
  Reported by:	Jon Passki
  MFC after:	1 week

Modified:
  head/etc/rc.d/tmp

Modified: head/etc/rc.d/tmp
==============================================================================
--- head/etc/rc.d/tmp	Sat Mar 13 11:51:18 2010	(r205120)
+++ head/etc/rc.d/tmp	Sat Mar 13 12:02:44 2010	(r205121)
@@ -51,8 +51,8 @@ case "${tmpmfs}" in
 [Nn][Oo])
 	;;
 *)
-	if /bin/mkdir -p /tmp/.diskless 2> /dev/null; then
-		rmdir /tmp/.diskless
+	if _tmpdir=$(mktemp -d -q /tmp/.diskless.XXXXXX); then
+		rmdir ${_tmpdir}
 	else
 		if [ -h /tmp ]; then
 			echo "*** /tmp is a symlink to a non-writable area!"
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Jaakko Heinonen freebsd_committer freebsd_triage 2010-03-13 12:20:08 UTC
State Changed
From-To: open->patched

Fixed in head (r205121).
Comment 7 dfilter service freebsd_committer freebsd_triage 2010-03-23 16:45:39 UTC
Author: jh
Date: Tue Mar 23 16:45:29 2010
New Revision: 205518
URL: http://svn.freebsd.org/changeset/base/205518

Log:
  MFC r205121:
  
  Use an unique directory name instead of hardcoded /tmp/.diskless.
  A malicious user could create a file named /tmp/.diskless and cause
  the script to misbehave.
  
  PR:		conf/141258

Modified:
  stable/8/etc/rc.d/tmp
Directory Properties:
  stable/8/etc/   (props changed)

Modified: stable/8/etc/rc.d/tmp
==============================================================================
--- stable/8/etc/rc.d/tmp	Tue Mar 23 15:14:45 2010	(r205517)
+++ stable/8/etc/rc.d/tmp	Tue Mar 23 16:45:29 2010	(r205518)
@@ -51,8 +51,8 @@ case "${tmpmfs}" in
 [Nn][Oo])
 	;;
 *)
-	if /bin/mkdir -p /tmp/.diskless 2> /dev/null; then
-		rmdir /tmp/.diskless
+	if _tmpdir=$(mktemp -d -q /tmp/.diskless.XXXXXX); then
+		rmdir ${_tmpdir}
 	else
 		if [ -h /tmp ]; then
 			echo "*** /tmp is a symlink to a non-writable area!"
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 dfilter service freebsd_committer freebsd_triage 2010-03-30 16:25:36 UTC
Author: jh
Date: Tue Mar 30 15:25:16 2010
New Revision: 205882
URL: http://svn.freebsd.org/changeset/base/205882

Log:
  MFC r205121:
  
  Use an unique directory name instead of hardcoded /tmp/.diskless.
  A malicious user could create a file named /tmp/.diskless and cause
  the script to misbehave.
  
  PR:		conf/141258

Modified:
  stable/7/etc/rc.d/tmp
Directory Properties:
  stable/7/etc/   (props changed)

Modified: stable/7/etc/rc.d/tmp
==============================================================================
--- stable/7/etc/rc.d/tmp	Tue Mar 30 13:55:15 2010	(r205881)
+++ stable/7/etc/rc.d/tmp	Tue Mar 30 15:25:16 2010	(r205882)
@@ -49,8 +49,8 @@ case "${tmpmfs}" in
 [Nn][Oo])
 	;;
 *)
-	if /bin/mkdir -p /tmp/.diskless 2> /dev/null; then
-		rmdir /tmp/.diskless
+	if _tmpdir=$(mktemp -d -q /tmp/.diskless.XXXXXX); then
+		rmdir ${_tmpdir}
 	else
 		if [ -h /tmp ]; then
 			echo "*** /tmp is a symlink to a non-writable area!"
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 dfilter service freebsd_committer freebsd_triage 2010-03-30 16:26:25 UTC
Author: jh
Date: Tue Mar 30 15:26:12 2010
New Revision: 205883
URL: http://svn.freebsd.org/changeset/base/205883

Log:
  MFC r205121:
  
  Use an unique directory name instead of hardcoded /tmp/.diskless.
  A malicious user could create a file named /tmp/.diskless and cause
  the script to misbehave.
  
  PR:		conf/141258

Modified:
  stable/6/etc/rc.d/tmp
Directory Properties:
  stable/6/etc/   (props changed)

Modified: stable/6/etc/rc.d/tmp
==============================================================================
--- stable/6/etc/rc.d/tmp	Tue Mar 30 15:25:16 2010	(r205882)
+++ stable/6/etc/rc.d/tmp	Tue Mar 30 15:26:12 2010	(r205883)
@@ -49,8 +49,8 @@ case "${tmpmfs}" in
 [Nn][Oo])
 	;;
 *)
-	if /bin/mkdir -p /tmp/.diskless 2> /dev/null; then
-		rmdir /tmp/.diskless
+	if _tmpdir=$(mktemp -d -q /tmp/.diskless.XXXXXX); then
+		rmdir ${_tmpdir}
 	else
 		if [ -h /tmp ]; then
 			echo "*** /tmp is a symlink to a non-writable area!"
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 10 Jaakko Heinonen freebsd_committer freebsd_triage 2010-03-30 16:44:49 UTC
State Changed
From-To: patched->closed

Fixed in head, stable/8, stable/7 and stable/6.