Hi,
Po Lu wrote:
> It has been brought to my attention that a defective memchr is
> distributed with certain Android E-book readers with Android 2.3.5 that
> is liable to integer overflows with large values of N:
>
> memchr ("", 0, SIZE_MAX);
This memchr invocation is wrong. memchr is defined in ISO C23 § 7.26.5.2
"The memchr generic function locates the first occurrence of c (...)
in the initial n characters (...) of the object pointed to by s."
The term "object" being defined in § 3.15:
"region of data storage in the execution environment, the contents
of which can represent values"
If you call memchr (s, c, n) where n > - (uintptr_t) s, you are pretending
that the NULL pointer is part of a region of data storage, which is wrong.
Similarly, if you call memchr (s, c, n) where n == - (uintptr_t) s, you are
pretending that the address (uintptr_t)(-1) is part of a region of data
storage, which is impossible on all OSes we know of.
Thus, it is the application's duty to provide a sane value for n. The
following code will usually avoid the overflow (although it still violates
the specification):
const char *s = "";
size_t n = SIZE_MAX;
if (n > ~(uintptr_t)s)
n = ~(uintptr_t)s;
memchr (s, 0, n);
> This extends to
> affect strnlen, which is implemented around memchr, ...
> producing crashes in this function in Emacs:
>
> static int
> store_mode_line_string (const char *string, Lisp_Object lisp_string,
> bool copy_string,
> int field_width, int precision, Lisp_Object props)
> {
> ptrdiff_t len;
> int n = 0;
>
> if (string != NULL)
> {
> len = strnlen (string, precision <= 0 ? SIZE_MAX : precision);
Similarly here. strnlen is specified by POSIX
https://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html
"The strnlen() function shall compute the smaller of the number of bytes
in the array to which s points, not including any terminating NUL character,
or the value of the maxlen argument. The strnlen() function shall never
examine more than maxlen bytes of the array pointed to by s."
In other words, if precision is 0, you are allowing the implementation to
look at SIZE_MAX bytes, which includes the NULL pointer. Thus the
implementation is allowed to crash.
> Perhaps Gnulib
> should provide a module with replacements for memchr and affected string
> functions, viz., strnlen, memmem, wmemchr, mbstowcs, mbsrtowcs,
> wcsrtombs, and wcstombs.
I think it is better if the Emacs code gets fixed. This will also eliminate
warnings from static analysis tools (e.g. "gcc -fanalyzer") that routinely
complain about an overflow when you pass SIZE_MAX as an object size or array
size.
Bruno
