bin/185393: find -lname buffer read overflow bug

[Ben Reser]

>Number:         185393
>Category:       bin
>Synopsis:       find -lname buffer read overflow bug
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Jan 01 19:40:00 UTC 2014
>Closed-Date:
>Last-Modified:
>Originator:     Ben Reser
>Release:        9.1
>Organization:
>Environment:
FreeBSD freebsd9.1 9.1-RELEASE FreeBSD 9.1-RELEASE #0 r243825: Tue Dec  4 09:23:10 UTC 2012     root at farrell.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC  amd64
>Description:
The implementation of -lname and -ilname improperly use readlink() by not setting a null character before using the string.  readlink() is documented as not doing this for you and returns the length of the link string, requiring the caller to set the null character.

In particular this is implemented in the usr.bin/find/function.c in the f_name() function.  The function uses an automatic buffer which gets reused through multiple calls, resulting in link names that are shorter than the preceding values stored in the buffer to fail to match properly.

This could cause the program to read past the end of the buffer.  In practice this doesn't seem to happen because the buffer seems to always end up in zeroed memory the first time it is used (though there's no requirement for it to do so).  This would result in a crash of the find command.

You can force reading past the end of the buffer by creating a link that points at a path of PATH_MAX length on the path being searched.  Presumably it's not possible to create a link that points at a path longer than that but if possible that would also allow reading past the end of the buffer.  I haven't bothered to exercise this.

It might be possible to view this as a minor security issue if someone is using find to try and find link with -lname for auditing purposes, since they might not reliably find what they are looking for.  The read past the end of the buffer doesn't seem particularly useful.  For one it'd only ever be a read, which isn't particularly useful and for another find doesn't run with escalated privileges.  So all in all I think it'd be a stretch to call this anything other than an ordinary bug.

This bug was introduced in r176497 (committed 5 years 10 months ago), so any releases of FreeBSD that contain this change would contain the same issue.  I actually happened to find the issue in OS X's fork of your find command.  But successfully duplicated the issue in a VM of 9.1 that I had laying around.
>How-To-Repeat:
The following shell script should demonstrate the issue:

#!/usr/bin/env bash

set -e

# Demonstration of -lname bug with FreeBSD and OS X find.

# find stops output matching links as soon as it passes a link
# that points at a path that is longer than the path we are trying
# to match.  Note that file system ordering of results may change
# when this happens.  OS X seems to return readdir results in
# alphabetical sorted order (HFS+) and FreeBSD (UFS) seems to return
# them in creation order (though there does seem to be some variation
# on this).  So the below example has both the creation
# order and the alphabetical sort order such that it should reliably
# reproduce the issue.  However, I've not tested this with other
# supported file systems so they may have different behavior, possibly
# even non-deterministic behavior that makes this harder to demonstrate.

# Expected behavior will have no output and a zero exit value.

test_dir=`mktemp -d find-test.XXXXXXX`
cd "$test_dir" > /dev/null
ln -s /usr/bin/gcc a
ln -s /usr/bin/touch b
ln -s /usr/bin/gcc c
ln -s /usr/bin/gcc d
ln -s /usr/bin/gcc e

echo './a' > expected
echo './c' >> expected
echo './d' >> expected
echo './e' >> expected

"${FIND:-find}" . -lname /usr/bin/gcc | sort > received
set +e
diff -u expected received
rv=$?
set -e
cd - > /dev/null
rm -rf "$test_dir"
exit $rv

>Fix:
Set a null character at fn[len] (where len is the return of the readlink() call) as implemented in the attached patch.

Patch attached with submission follows:

Index: usr.bin/find/function.c
===================================================================
--- usr.bin/find/function.c	(revision 260159)
+++ usr.bin/find/function.c	(working copy)
@@ -1124,9 +1124,11 @@ f_name(PLAN *plan, FTSENT *entry)
 	const char *name;
 
 	if (plan->flags & F_LINK) {
+		int len = readlink(entry->fts_path, fn, sizeof(fn));
+		if (len == -1)
+			return 0;
+		fn[len] = '\0';
 		name = fn;
-		if (readlink(entry->fts_path, fn, sizeof(fn)) == -1)
-			return 0;
 	} else
 		name = entry->fts_name;
 	return !fnmatch(plan->c_data, name,


>Release-Note:
>Audit-Trail:
>Unformatted: