Be more careful calling wcstombs

The function is not guaranteed to NUL-terminate its output.  It
*is*, however, guaranteed not to generate more than two bytes per
multibyte character (plus terminating nul), so the general approach
I'm taking is to try to allocate enough space, AND to manually add a
NUL at the end of each buffer just in case I screwed up the "enough
space" thing.

Fixes bug 5909.

changes/bug5909

@@ -0,0 +1,5 @@
+  o Major bugfixes:
+    - When building Tor on Windows with -DUNICODE (not default),
+      ensure that error messages, filenames, and DNS server names are
+      always NUL-terminated when we convert them to a single-byte
+      encoding.  Fixes bug 5909; bugfix on 0.2.2.16-alpha.

src/common/compat.c

@@ -3046,9 +3046,10 @@ format_win32_error(DWORD err)
 {
   TCHAR *str = NULL;
   char *result;
+  DWORD n;
 
   /* Somebody once decided that this interface was better than strerror(). */
-  FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+  n = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
                  FORMAT_MESSAGE_FROM_SYSTEM |
                  FORMAT_MESSAGE_IGNORE_INSERTS,
                  NULL, err,
@@ -3056,18 +3057,26 @@ format_win32_error(DWORD err)
                  (LPVOID)&str,
                  0, NULL);
 
-  if (str) {
+  if (str && n) {
 #ifdef UNICODE
-    char abuf[1024] = {0};
+    size_t len;
-    wcstombs(abuf,str,1024);
+    if (n > 128*1024)
-    result = tor_strdup(abuf);
+      len = (128 * 1024) * 2 + 1; /* This shouldn't be possible, but let's
+                                   * make sure. */
+    else
+      len = n * 2 + 1;
+    result = tor_malloc(len);
+    wcstombs(result,str,len);
+    result[len-1] = '\0';
 #else
     result = tor_strdup(str);
 #endif
-    LocalFree(str); /* LocalFree != free() */
   } else {
     result = tor_strdup("<unformattable error>");
   }
+  if (str) {
+    LocalFree(str); /* LocalFree != free() */
+  }
   return result;
 }
 #endif

src/common/util.c

@@ -2855,7 +2855,7 @@ tor_listdir(const char *dirname)
 #ifdef _WIN32
   char *pattern=NULL;
   TCHAR tpattern[MAX_PATH] = {0};
-  char name[MAX_PATH] = {0};
+  char name[MAX_PATH*2+1] = {0};
   HANDLE handle;
   WIN32_FIND_DATA findData;
   tor_asprintf(&pattern, "%s\\*", dirname);
@@ -2872,6 +2872,7 @@ tor_listdir(const char *dirname)
   while (1) {
 #ifdef UNICODE
     wcstombs(name,findData.cFileName,MAX_PATH);
+    name[sizeof(name)-1] = '\0';
 #else
     strlcpy(name,findData.cFileName,sizeof(name));
 #endif

src/or/config.c

@@ -4302,7 +4302,7 @@ static char *
 get_windows_conf_root(void)
 {
   static int is_set = 0;
-  static char path[MAX_PATH+1];
+  static char path[MAX_PATH*2+1];
   TCHAR tpath[MAX_PATH] = {0};
 
   LPITEMIDLIST idl;
@@ -4332,7 +4332,8 @@ get_windows_conf_root(void)
   /* Convert the path from an "ID List" (whatever that is!) to a path. */
   result = SHGetPathFromIDList(idl, tpath);
 #ifdef UNICODE
-  wcstombs(path,tpath,MAX_PATH);
+  wcstombs(path,tpath,sizeof(path));
+  path[sizeof(path)-1] = '\0';
 #else
   strlcpy(path,tpath,sizeof(path));
 #endif

src/or/eventdns.c

@@ -3226,6 +3226,7 @@ config_nameserver_from_reg_key(HKEY key, const TCHAR *subkey)
 	if (RegQueryValueEx(key, subkey, 0, &type, (LPBYTE)buf, &bufsz)
 		== ERROR_SUCCESS && bufsz > 1) {
 		wcstombs(ansibuf,(wchar_t*)buf,MAX_PATH);/*XXXX UNICODE */
+		abuf[MAX_PATH-1] = '\0';
 		status = evdns_nameserver_ip_add_line(ansibuf);
 	}
 

src/or/ntmain.c

@@ -455,7 +455,7 @@ static char *
 nt_service_command_line(int *using_default_torrc)
 {
   TCHAR tor_exe[MAX_PATH+1];
-  char tor_exe_ascii[MAX_PATH+1];
+  char tor_exe_ascii[MAX_PATH*2+1];
   char *command=NULL, *options=NULL;
   smartlist_t *sl;
   int i;
@@ -483,6 +483,7 @@ nt_service_command_line(int *using_default_torrc)
 
 #ifdef UNICODE
   wcstombs(tor_exe_ascii, tor_exe, sizeof(tor_exe_ascii));
+  tor_exe_ascii[sizeof(tor_exe_ascii)-1] = '\0';
 #else
   strlcpy(tor_exe_ascii, tor_exe, sizeof(tor_exe_ascii));
 #endif