Fix CID 1006692 in /usr/sbin/pw pw_log() function and other fixes

[truckman]

Summary

Limit the length of the name retrieved from the environment
variable $LOGNAME or $USER. Sanitize it by removing whitespace
to benefit log file parsers. Double any embedded '%' characters
since this string will be embedded in a *printf format string.

Detect and warn about overflow when building the format string.

[markj]

I think this looks right from a correctness perspective. It might be nice to have a comment explaining what sanitization we're doing. The amount of indentation in this function could be reduced if we return early in the logfile == NULL case.

usr.sbin/pw/pw_log.c

63

The indentation looks wonky here, but that may just be phabricator.

67

The cast seems superfluous since we're comparing with a constant whose value is clearly representable with an int. Does clang/coverity warn about this?

(ditto below)

69

Why not have cp++ in the loop header?

[truckman]

I like the early return idea, though it will make the diff a lot bigger.

usr.sbin/pw/pw_log.c

63

I tried to match the existing style here with outdented "*" for pointers.

67

Yes, clang complains, which I agree is bogus in this case:

cc -O2 -pipe -g -MD -MF.depend.pw_log.o -MTpw_log.o -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c /usr/src/usr.sbin/pw/pw_log.c -o pw_log.o
/usr/src/usr.sbin/pw/pw_log.c:67:11: error: comparison of integers of different

signs: 'int' and 'unsigned long' [-Werror,-Wsign-compare]
                              i < sizeof(sname) - 1; ) {
                              ~ ^ ~~~~~~~~~~~~~~~~~

1 error generated.

69

Good idea.

[truckman]

Return early when possible to reduce code nesting.

Add a comment to explain the sanitization of name.

Increment cp at the top of the for loop.

Adjust variable declarations to be more style compliant, including
moving initialization out of the declarations.

[truckman]

Re-order the tests in the name sanitization loop to eliminate the else clause

[truckman]

Whitespace changes

[markj]

LGTM, thanks! My only other nit is that it might be nice to include the errant username in the overflow warning message.