Hi,
Some of nmh's code uses strcpy(3). That can overflow the destination.
Probably for that reason, some uses strncpy(3), e.g.
char s[BUFSIZ];
strncpy(s, addr, sizeof s);
That avoids the overflow, but s may lack a NUL terminator. So some code
goes one better with
strncpy(s, addr, sizeof s - 1);
s[sizeof s - 1] = '\0';
This leaves s NUL terminated, but possibly silently truncated. Also, if
addr is a lot shorter than BUFSIZ, 8KiB here, then strncpy NULs out all
the rest of the 8KiB past the end of the string's terminator NUL. I'd
think that's not needed by most of the callers, though it's difficult to
know without examining each case. And there's quite a bit of repetition
now, `s' four times, `s - 1' twice, and they need checking for
consistency. `s' is often a bit more complex than here. If it's not
sizeof s is that a cut-and-paste error, or deliberate?
(strncpy's role isn't to truncate particularly, it's to copy a string to
a zero-padded fixed-length record, e.g. the 14-char filename in a
directory entry that has a two-byte inode number alongside.)
I was thinking of switching to strlcpy(3) from BSD, also available on
Linux with -lbsd or similar. A stand-in if it's not available is
trivial. The autoconf less so, but seemingly do-able.
http://lists.windowmaker.org/dev/msg01783.html
«snprintf(s, sizeof s, "%s", addr)» is the equivalent. Both would
shorten the code back to one line, so less to review, but both would
still silently truncate.
So how about our own function that takes (dest, src, size) and if
strlen(src) isn't less than size then it abort(3)s. So, it's a checking
strcpy, no more. A macro for the common case of size being dest's
sizeof will remove the last bit of repetition.
It won't be a correct substitute if the zero-padding, or the truncation
to a fixed-width buffer were required, but that's a minority of uses, I
expect. And aborting in a library routine would normally be unfriendly,
but we're not expecting them too, and it would be nice to flush out the
few.
Yes, I know a borrowed data-type string library, or our own, would seem
ideal, but I'm trying to enable a lot of small step improvements as I
don't think a grand-design solution will get done with our limited
resources. Once the low-hanging strncpy's were switched over, the few
remaining ones would be easier to examine to understand why they truly
need to be strncpy.
--
Cheers, Ralph.
https://plus.google.com/+RalphCorderoy
_______________________________________________
Nmh-workers mailing list
Nmh-workers(_at_)nongnu(_dot_)org
https://lists.nongnu.org/mailman/listinfo/nmh-workers