cygwin wcsxfrm: byte swap result ourselves

Workaround a bug (or undocumented behaviour) in LCMapStringW:

It's documented(*) that the cchDest parameter is a byte count with
LCMAP_SORTKEY, but a character count otherwise.  But the docs don't
state what happens if you combine LCMAP_SORTKEY with LCMAP_BYTEREV.

Tests indicate that LCMAP_SORTKEY treats cchDest as byte count, but
then LCMAP_BYTEREV treats it as char count in the same call.  So the
latter swaps twice as much bytes in the destination buffer than the
byte count it returns, which potentially results in writing past the
end of the given output buffer.

Solution: Don't specify LCMAP_BYTEREV in the LCMapStringW(LCMAP_SORTKEY)
call, rather byte swap afterwards.

(*) https://msdn.microsoft.com/en-us/library/windows/desktop/dd318702(v=vs.85).aspx

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

1 files changed, 13 insertions, 8 deletions

diff --git a/winsup/cygwin/nlsfuncs.cc b/winsup/cygwin/nlsfuncs.cc
index 60ad2d375..5bf7d22ac 100644
--- a/winsup/cygwin/nlsfuncs.cc
+++ b/winsup/cygwin/nlsfuncs.cc
@@ -1196,15 +1196,21 @@ wcsxfrm_l (wchar_t *__restrict ws1, const wchar_t *__restrict ws2, size_t wsn,
 
   if (!collate_lcid)
     return wcslcpy (ws1, ws2, wsn);
-  ret = LCMapStringW (collate_lcid, LCMAP_SORTKEY | LCMAP_BYTEREV,
-		      ws2, -1, ws1, wsn * sizeof (wchar_t));
-  /* LCMapStringW returns byte count including the terminating NUL character,
-     wcsxfrm is supposed to return length in wchar_t excluding the NUL.
-     Since the array is only single byte NUL-terminated we must make sure
-     the result is wchar_t-NUL terminated. */
+  /* Don't use LCMAP_SORTKEY in conjunction with LCMAP_BYTEREV.  The cchDest
+     parameter is used as byte count with LCMAP_SORTKEY but as char count with
+     LCMAP_BYTEREV. */
+  ret = LCMapStringW (collate_lcid, LCMAP_SORTKEY, ws2, -1, ws1,
+		      wsn * sizeof (wchar_t));
   if (ret)
     {
       ret /= sizeof (wchar_t);
+     /* Byte swap the array ourselves here. */
+      for (size_t idx = 0; idx < ret; ++idx)
+	ws1[idx] = __builtin_bswap16 (ws1[idx]);
+      /* LCMapStringW returns byte count including the terminating NUL char.
+	 wcsxfrm is supposed to return length in wchar_t excluding the NUL.
+	 Since the array is only single byte NUL-terminated yet, make sure
+	 the result is wchar_t-NUL terminated. */
       if (ret < wsn)
 	ws1[ret] = L'\0';
       return ret;
@@ -1213,8 +1219,7 @@ wcsxfrm_l (wchar_t *__restrict ws1, const wchar_t *__restrict ws2, size_t wsn,
     set_errno (EINVAL);
   else
     {
-      ret = LCMapStringW (collate_lcid, LCMAP_SORTKEY | LCMAP_BYTEREV, ws2, -1,
-			  NULL, 0);
+      ret = LCMapStringW (collate_lcid, LCMAP_SORTKEY, ws2, -1, NULL, 0);
       if (ret)
 	wsn = ret / sizeof (wchar_t);
     }