[ruby-core:85464] [Ruby trunk Bug#14453] Crash in w32_cmdvector() if MS Application Verifier is enabled

Issue #14453 has been updated by PetrH (Petr Hluzin).


Thanks for the link, the change in git indeed looks like a fix.
I will try the new version and let you know.

----------------------------------------
Bug #14453: Crash in w32_cmdvector() if MS Application Verifier is enabled
https://bugs.ruby-lang.org/issues/14453#change-70255

* Author: PetrH (Petr Hluzin)
* Status: Closed
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.4.1p111 (2017-03-22) [i386-mswin32_140]
* Backport: 2.3: REQUIRED, 2.4: REQUIRED, 2.5: DONTNEED
----------------------------------------
On Windows, Application Verifier (AV) is a tool by Microsoft that detects common bugs in applications. When AV detects a bug, it usually crashes the process, depending on a kind of the bug.

Steps to reproduce:
1. Enable verification of heap-related calls in Application Verifier
2. Launch ruby.exe with no parameters. The crash occurs before any script is loaded.

Result:

```
Process ruby.exe crashes with following functions on call-stack:
00 vcruntime140_ruby240!strlcpy
01 vcruntime140_ruby240!w32_cmdvector
02 vcruntime140_ruby240!rb_w32_sysinit
03 vcruntime140_ruby240!ruby_sysinit
04 ruby!main
05 ruby!invoke_main
06 ruby!__scrt_common_main_seh
07 kernel32!BaseThreadInitThunk
08 ntdll!__RtlUserThreadStart
09 ntdll!_RtlUserThreadStart
```

Expected result:
The process should not crash and show REPL prompt or something.

The Application Verifier is a GUI tool. Instead you can use gflags.exe tool that comes with a Debugging Tools for Windows and invoke "gflags.exe /p /enable ruby.exe /full". "Full page heap" is needed (the default debugging mode) for the crash to occur.

The crash occurs because line in function w32_cmdvector() in src\win32\win32.c

```c
	curr->str = rb_w32_wstr_to_mbstr(cp, base, len, &curr->len);
```

allocates space and does _not_ terminate string `curr->str'.
When later function `w32_cmdvector()` executes line

```c
	strlcpy(cptr, curr->str, curr->len + 1);
```

the implementation of function `strlcpy()` in src\missing\strlcpy.c copies the needed bytes and then traverses following characters until terminating NUL byte (here missing), apparently in hope of detecting an unterminated string.

When heap verification in Application Verifier is enabled, then a non-accessible page is placed after each allocation, similar to how famous Electric Fence works. The `strlcpy()` runs into the non-accessible page and triggers access violation.

Suggested fix (three line):

```diff
 char *
 rb_w32_wstr_to_mbstr(UINT cp, const WCHAR *wstr, int clen, long *plen)
 {
     char *ptr;
     int len = WideCharToMultiByte(cp, 0, wstr, clen, NULL, 0, NULL, NULL);
-    if (!(ptr = malloc(len))) return 0;
+    if (!(ptr = malloc((clen == -1) ? len : (len + 1)))) return 0;
     WideCharToMultiByte(cp, 0, wstr, clen, ptr, len, NULL, NULL);
     if (plen) {
 	/* exclude NUL only if NUL-terminated string */
 	if (clen == -1) --len;
 	*plen = len;
     }
+    if (clen != -1)
+	ptr[len] = '\0';
     return ptr;
 }
```

This fix also has the minor advantage that this line in `w32_cmdvector()`

```c
	strlcpy(cptr, curr->str, curr->len + 1);
```

can be changed to a simple `strcpy()`. Or the `strlcpy()` call can be changed in a way the third parameter will be the remaining capacity of buffer, as it is the purpose of `strlcpy()`.




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>