Created attachment 165469 [details] Crashing testcase The wcslcat function could read several bytes behind the end of its input buffer. This could lead to a crash if the buffer happens to immediately precede an unmapped page (or when dst=NULL and n=0). The strlcat function[1] and, hence[2], wcslcat function are documented to work with the destination buffer not containing NUL. In this case FreeBSD implementation of wcslcat will read one extra wide char. The code[3] for traversing the destination array: 56 /* Find the end of dst and adjust bytes left but don't go past end */ 57 while (*d != '\0' && n-- != 0) 58 d++; "n-- != 0" in the loop controlling expression makes sure that the loop is terminated after n chars are examined but the dereference in "*d != '\0'" happens before the "n" check. For example, there would be one dereference when n=0. In particular, wcslcat(NULL, L"", 0) will crash. A crashing testcase with non-null dst is attached. To fix it, it's enough to swap the checks in the while loop (patch attached). Or all the code could be changed to match strlcat. The issue has security consequences but the function is rarely used so severity seems very low. Other BSDs are affected except for OpenBSD which fixed it in [4]. The issue is similar to [5]. [1] https://svnweb.freebsd.org/base/head/lib/libc/string/strlcpy.3?revision=257720&view=markup#l86 [2] https://svnweb.freebsd.org/base/head/lib/libc/string/wmemchr.3?revision=251069&view=markup [3] https://svnweb.freebsd.org/base/head/lib/libc/string/wcslcat.c?revision=188080&view=markup#l56 [4] http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/wcslcat.c?rev=1.4&content-type=text/x-cvsweb-markup [5] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206177
Created attachment 165470 [details] Patch
Great catch! I'll get this committed.
A commit references this bug: Author: brooks Date: Wed Jan 13 21:50:09 UTC 2016 New revision: 293856 URL: https://svnweb.freebsd.org/changeset/base/293856 Log: Avoid reading pass the end of the source buffer when it is not NUL terminated. If this buffer is adjacent to an unmapped page or a version of C with bounds checked is used this may result in a crash. PR: 206178 Submitted by: Alexander Cherepanov <cherepan@mccme.ru> MFC after: 1 week Changes: head/lib/libc/string/wcslcat.c
A commit references this bug: Author: brooks Date: Wed Jan 20 19:26:05 UTC 2016 New revision: 294455 URL: https://svnweb.freebsd.org/changeset/base/294455 Log: MFC r293856: Avoid reading pass the end of the source buffer when it is not NUL terminated. If this buffer is adjacent to an unmapped page or a version of C with bounds checked is used this may result in a crash. PR: 206178 Submitted by: Alexander Cherepanov <cherepan@mccme.ru> Changes: _U stable/10/ stable/10/lib/libc/string/wcslcat.c
A commit references this bug: Author: brooks Date: Wed Jan 20 19:56:44 UTC 2016 New revision: 294457 URL: https://svnweb.freebsd.org/changeset/base/294457 Log: MFC r293856: Avoid reading pass the end of the source buffer when it is not NUL terminated. If this buffer is adjacent to an unmapped page or a version of C with bounds checked is used this may result in a crash. PR: 206178 Submitted by: Alexander Cherepanov <cherepan@mccme.ru> Changes: _U stable/9/lib/libc/ stable/9/lib/libc/string/wcslcat.c
A commit references this bug: Author: brooks Date: Fri Jan 22 00:13:18 UTC 2016 New revision: 294538 URL: https://svnweb.freebsd.org/changeset/base/294538 Log: MFC r293856: Avoid reading pass the end of the source buffer when it is not NUL terminated. If this buffer is adjacent to an unmapped page or a version of C with bounds checked is used this may result in a crash. PR: 206178 Submitted by: Alexander Cherepanov <cherepan@mccme.ru> Requested by: danfe Changes: _U stable/8/lib/libc/ stable/8/lib/libc/string/wcslcat.c