bin/132367: {less, more}(1) fchmod(2) `/dev/null' to 0600 under certain conditions

[Lucio Andrés Illanes Albornoz]

>Number:         132367
>Category:       bin
>Synopsis:       {less,more}(1) fchmod(2) `/dev/null' to 0600 under certain conditions
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Mar 06 17:00:11 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Lucio Albornoz
>Release:        FreeBSD 7.1-RELEASE i386
>Organization:
>Environment:
System: FreeBSD aynur.local 7.1-RELEASE FreeBSD 7.1-RELEASE #0: Thu Jan  1 14:37:25 UTC 2009     root at logan.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC  i386
>Description:
{less,more}(1) from the contrib/ tree implement command history saving
which occurs on program exit if the latter has been touched and modified.
Presence of a path name in the `LESSHISTFILE' environment variable pointing
to a file or, in case the variable isn't set or empty, the fallback default
of `${HOME}/.lesshst' (the former overriding the latter,) will direct
the program to
1) fchmod(2) the file to 0600 for security reasons, and
2) Save the non-empty command history.
Only `/dev/null' being set in the environment variable will prevent both.
However, if `${HOME}/.lesshst' is a symbolic link pointing to `/dev/null',
then {less,more}(1) running under superuser credentials will render the
null(4) character device file useless for everyone else.

The attached patch introduces special-case semantics for the last case of
the default file name being a symbolic link to `/dev/null' by skipping
command history saving entirely.
>How-To-Repeat:
# ln -sf /dev/null ~/.lesshst
# less -f /dev/null            # Or any other file
/null                          # Or any other command
q
#                              # /dev/null would be 0600'd by now
>Fix:
This here patch, applied within src/usr.bin/less:

--- cmdbuf.c.orig	2009-03-06 16:53:03.151960882 +0100
+++ cmdbuf.c	2009-03-06 17:23:56.884968001 +0100
@@ -1324,6 +1324,8 @@
 	char *home;
 	char *name;
 	int len;
+	char devnull_name[10];
+	size_t devnull_size = sizeof(devnull_name);
 	
	/* See if filename is explicitly specified by $LESSHISTFILE. */
	name = lgetenv("LESSHISTFILE");
@@ -1348,6 +1350,11 @@
	len = strlen(home) + strlen(LESSHISTFILE) + 2;
	name = (char *) ecalloc(len, sizeof(char));
	SNPRINTF2(name, len, "%s/%s", home, LESSHISTFILE);
+
+	if(readlink(name, &(devnull_name[0]), devnull_size) == (devnull_size - 1) &&
+	   strncmp(&(devnull_name[0]), "/dev/null", (devnull_size - 1)) == 0)
+		return (NULL);
+
 	return (name);
 }
 #endif /* CMD_HISTORY */
>Release-Note:
>Audit-Trail:
>Unformatted: