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.

diff --git a/changes/bug5909 b/changes/bug5909
new file mode 100644 (file)
index 0000000..61990fa
--- /dev/null
+++ b/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.

diff --git a/src/common/compat.c b/src/common/compat.c
index 334ea1b8b404d6920beedee200564cfabf4a6318..db2187b836f94bf88f23e732ae84568ecb08ec18 100644 (file)
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -3046,28 +3046,37 @@ 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,
                  MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-                (LPVOID)&str,
+                 (LPVOID)&str,
                  0, NULL);
 
-  if (str) {
+  if (str && n) {
 #ifdef UNICODE
-    char abuf[1024] = {0};
-    wcstombs(abuf,str,1024);
-    result = tor_strdup(abuf);
+    size_t len;
+    if (n > 128*1024)
+      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

diff --git a/src/common/util.c b/src/common/util.c
index 4c086e86fce6d93ea5bd249727923809f8f0c97b..dc8f8b79957fea7c78620f54f89b3ba03dcb2d22 100644 (file)
--- a/src/common/util.c
+++ b/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

diff --git a/src/or/config.c b/src/or/config.c
index 090d96c155cec728daad2f845d225dd3ab9e53ba..2e3ce6429dce4d86f911269152c10076e1f59f8c 100644 (file)
--- a/src/or/config.c
+++ b/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

diff --git a/src/or/eventdns.c b/src/or/eventdns.c
index 61a28361abdc3c03339c4194d52113544ec13982..768693aba63b81f1a63a470d3381fe36bcb65f8b 100644 (file)
--- a/src/or/eventdns.c
+++ b/src/or/eventdns.c
@@ -3213,7 +3213,7 @@ static int
 config_nameserver_from_reg_key(HKEY key, const TCHAR *subkey)
 {
        char *buf;
-  char ansibuf[MAX_PATH] = {0};
+       char ansibuf[MAX_PATH] = {0};
        DWORD bufsz = 0, type = 0;
        int status = 0;
 
@@ -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);
        }
 

diff --git a/src/or/ntmain.c b/src/or/ntmain.c
index 01244c4c08933995aa9f6fe7906338b61223985d..d001f7be1363e55cbbb1a489cc3c1cbd3008b8a1 100644 (file)
--- a/src/or/ntmain.c
+++ b/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