Issue #197: fixed buffer overflow in `_makeWchar()`
...when passed string is `Unicode16String`. It (wrongly) used
passed buffer size as size of the buffer in character while
caller passed it as a size in bytes (a result of `sizeof()`).
This naturally caused a buffer overflow when size of (unicode)
string was greater (MAXPATHLEN / 2), causing the segmentation
violation.
This commit fixes this mismatch and makes sure that once
`_makeWchar()` returns, it's properly NULL terminated.
https://swing.fit.cvut.cz/projects/stx-jv/ticket/197
--- a/Win32OperatingSystem.st Mon Mar 05 10:46:56 2018 +0000
+++ b/Win32OperatingSystem.st Wed Mar 28 09:51:58 2018 +0100
@@ -586,28 +586,45 @@
!Win32OperatingSystem primitiveFunctions!
%{
-int
-_makeWchar(OBJ string, wchar_t *buffer, int bufferSize)
+
+/**
+ * The `_makeWchar()` function copies contents of a string object `srcObj` to a
+ * (wide char) buffer `dst`. At most `n` **bytes** including null terminator
+ * are copied to destination buffer `dst`.
+ *
+ * Returns the number of **bytes** copied (including null terminator). If
+ * source string object is not an instance of `String` or `Unicode16String`,
+ * returns `-1`.
+ */
+static int
+_makeWchar(OBJ srcObj, wchar_t *dst, int n/* in bytes!!! */)
{
- int i, len;
-
- if (__isStringLike(string)) {
- len = __stringSize(string);
- if (len > bufferSize) len = bufferSize;
- for (i=0; i<len; i++) {
- buffer[i] = __stringVal(string)[i];
- }
- } else if (__isUnicode16String(string)) {
- len = __unicode16StringSize(string);
- if (len > bufferSize) len = bufferSize;
- for (i=0; i<len; i++) {
- buffer[i] = __unicode16StringVal(string)[i];
- }
+ int i;
+ int srcLen; /* length of string to be copied, in **characters** */
+ int dstLen; /* length of destination buffer, in **characters** */
+
+ dstLen = n / sizeof(wchar_t);
+
+ if (__isStringLike(srcObj)) {
+ srcLen = __stringSize(srcObj);
+
+ if (srcLen >= dstLen) srcLen = dstLen - 1;
+
+ for (i=0; i<srcLen; i++) {
+ dst[i] = __stringVal(srcObj)[i];
+ }
+ } else if (__isUnicode16String(srcObj)) {
+ srcLen = __unicode16StringSize(srcObj);
+
+ if (srcLen >= dstLen) srcLen = dstLen - 1;
+
+ memcpy(dst, __unicode16StringVal(srcObj), srcLen * sizeof(wchar_t));
} else {
+ dst[0] = 0;
return(-1);
}
- buffer[len] = 0;
- return(len);
+ dst[srcLen] = 0;
+ return(srcLen * sizeof(wchar_t));
}
@@ -6700,7 +6717,7 @@
if (__isSmallInteger(anInteger)) {
if (__isStringLike(aPathName)) {
#ifdef DO_WRAP_CALLS
- char _aPathName[MAXPATHLEN];
+ char _aPathName[MAXPATHLEN+1];
strncpy(_aPathName, __stringVal(aPathName), MAXPATHLEN-1); _aPathName[MAXPATHLEN-1] = '\0';
do {