>Number:         5071
>Category:       bin
>Synopsis:       "lpq user" causes segmentation fault
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Feb 26 11:05:01 1998
>Last-Modified:
>Originator:     Havard Eidnes
>Organization:
	RUNIT A/S
>Release:        1.3_ALPHA, user-land Dec 3 1997 vintage
>Environment:
System: NetBSD vader.runit.sintef.no 1.3_ALPHA NetBSD 1.3_ALPHA (VADER) #11: Sat Nov 8 20:20:41 MET 1997 he@vader.runit.sintef.no:/usr/src/sys/arch/i386/compile/VADER i386


>Description:
	Doing "lpq user" causes a segmentation fault.
>How-To-Repeat:
	See above.
>Fix:
	No fix supplied here, although I can point at the piece of the
	code where this bug is to be found.  Specifically,
	/usr/src.usr.sbin/lpr/common_source/displayq.c contains this sequence
	of code:

        (void)snprintf(line, sizeof(line), "%c%s", format + '\3', RP);
        cp = line;
        for (i = 0; i < requests && cp-line+10 < sizeof(line) - 1; i++) {
                cp += strlen(cp);
                (void)snprintf(cp, line - cp, " %d", requ[i]);
        }
        for (i = 0; i < users && cp - line + 1 + strlen(user[i]) <
            sizeof(line) - 1; i++) {
                cp += strlen(cp);
                if (cp - line > sizeof(line) - 1)
                        break;
                *cp++ = ' ';
                (void)strncpy(cp, user[i], line - cp - 1);
        }

	The problem here is that 'cp' ends up as larger than 'line', and
	this causes "line - cp - 1" to take a small negative value.  This
	in turn is implicitly cast via the "size_t" argument type of strncpy,
	which on i386 is "unsigned int", and it is downhill from there (it
	crashes in the last strncpy() call.

	I would have thought that the expression

		min(sizeof(line) - (cp - line) - 1, strlen(user[i])

	should be used as the last argument to the strncpy call.

	With the attached patch at least lpq no longer segfaults in this
	situation.  I'm sure a stylistically prettier solution can be found.

--- displayq.c.dist	Mon Oct  6 13:34:01 1997
+++ displayq.c	Thu Feb 26 14:01:33 1998
@@ -244,5 +244,8 @@
 			break;
 		*cp++ = ' ';
-		(void)strncpy(cp, user[i], line - cp - 1);
+#define min(a,b) ((a)<(b) ? (a) : (b))
+		(void)strncpy(cp, user[i], 
+			min(sizeof(line) - (cp-line) - 1, strlen(user[i])));
+#undef min
 	}
 	(void)strncat(line, "\n", sizeof(line) - strlen(line) - 1);
>Audit-Trail:
>Unformatted: