[Bug string/19391] strnlen invokes UB by adding maxlen to str

["cherepan at mccme dot ru" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=19391

--- Comment #3 from Alexander Cherepanov <cherepan at mccme dot ru> ---
(In reply to Florian Weimer from comment #1)
> Isn't strnlen (p, -1) equivalent to strlen (p)?  It's difficult to tell from
> the specification.

1. I'm not sure what you mean. IMHO, yes, according to the
specifications[1][2], strnlen(p, -1) is equivalent to strlen(p) (modulo their
behavior on an array of SIZE_MAX non-NUL bytes and if objects can't be larger
than SIZE_MAX bytes). There is no explicit language requiring accesses to all
bytes up to maxlen to be valid hence an implementation can't assume it. And
IIUC this bug report filed based on this POV.

Is there a chance that strnlen("abc", -1) and, by generalization, strncmp("ab",
"abc", 5) are considered invalid?

Anyway, strnlen(p, -1) is in glibc tests[3].

The glibc manual is indeed silent on some aspects of the strn* functions -- I
files pr19407 (and pr19406 while at it). But it's not applicable to strnlen.

[1]
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html#tag_16_577_03
[2]
https://www.gnu.org/software/libc/manual/html_node/String-Length.html#index-strnlen
[3]
https://sourceware.org/git/?p=glibc.git;a=blob;f=string/tst-strlen.c;h=8376831de559c12057a7a432dc91953fb4e9624f;hb=HEAD#l43

2. No matter what, a comparison of the form "buffer + len < buffer" (where
buffer is a pointer and len is a non-negative integer) is always a sign of
problems. According to the C standard it's either false or UB. If the code does
something when the result of the comparison is true it means two things:
1) such a case is expected;
2) the code to handle the case is invalid (invokes UB). 

The straight comparison "buffer + len < buffer" is optimized into
"(ptrdiff_t)len < 0" right now by gcc (and by clang) which is not what a
programmer intended. The variant with an intermediary variable -- "end_ptr =
buffer + len; if (end_ptr < buffer)..." -- seems not to be optimized by current
gcc (but is optimized by clang). To depend on this is not the safest thing to
do. Please note that the current tests with n=SIZE_MAX will not catch this
optimization (because (ptrdiff_t)SIZE_MAX < 0).

Overall, very handy and robust way to find invalid pointer arithmetic. But, to
iterate, the comparison itself is only a sign of it and it's not enough to fix
the comparison alone.

3. I guess (and hope) that, in the end, this is considered a bug by the glibc
community even if a very low priority one (but who knows, given that "the GNU C
Library is implemented in GNU C, not in ISO C"). So I went and filed several
more similar bugs: pr19411, pr19412, pr19413.

-- 
You are receiving this mail because:
You are on the CC list for the bug.