Fix dodgy strcats in access_random_seed().

Looking over this function today, I spotted several questionable uses
of strcat to concatenate "\PUTTY.RND" to the end of a pathname,
without having checked whether the pathname had filled up the static
fixed-size buffer already.

I don't think this is exploitable (because you'd have to be in control
of the local account already to control any of the data sources used
to fill those buffers). But it's horrible anyway, of course. Now all
of those are replaced with sensible dupcats.

(This patch re-indents a lot of the function, to give variables
tighter scopes. So the diff is best viewed with whitespace ignored.)

windows/winstore.c

@@ -488,12 +488,17 @@ static bool try_random_seed(char const *path, int action, HANDLE *ret)
     return (*ret != INVALID_HANDLE_VALUE);
 }
 
+static bool try_random_seed_and_free(char *path, int action, HANDLE *hout)
+{
+    bool retd = try_random_seed(path, action, hout);
+    sfree(path);
+    return retd;
+}
+
 static HANDLE access_random_seed(int action)
 {
     HKEY rkey;
-    DWORD type, size;
     HANDLE rethandle;
-    char seedpath[2 * MAX_PATH + 10] = "\0";
 
     /*
      * Iterate over a selection of possible random seed paths until
@@ -510,17 +515,18 @@ static HANDLE access_random_seed(int action)
      * First, try the location specified by the user in the
      * Registry, if any.
      */
-    size = sizeof(seedpath);
+    {
-    if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_POS, &rkey) ==
+        char regpath[MAX_PATH + 1];
-	ERROR_SUCCESS) {
+        DWORD type, size = sizeof(regpath);
-	int ret = RegQueryValueEx(rkey, "RandSeedFile",
+        if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_POS, &rkey) ==
-				  0, &type, (BYTE *)seedpath, &size);
+            ERROR_SUCCESS) {
-	if (ret != ERROR_SUCCESS || type != REG_SZ)
+            int ret = RegQueryValueEx(rkey, "RandSeedFile",
-	    seedpath[0] = '\0';
+                                      0, &type, (BYTE *)regpath, &size);
-	RegCloseKey(rkey);
+            RegCloseKey(rkey);
-
+            if (ret == ERROR_SUCCESS && type == REG_SZ &&
-	if (*seedpath && try_random_seed(seedpath, action, &rethandle))
+                try_random_seed(regpath, action, &rethandle))
-	    return rethandle;
+                return rethandle;
+        }
     }
 
     /*
@@ -541,19 +547,20 @@ static HANDLE access_random_seed(int action)
 	tried_shgetfolderpath = true;
     }
     if (p_SHGetFolderPathA) {
+        char profile[MAX_PATH + 1];
 	if (SUCCEEDED(p_SHGetFolderPathA(NULL, CSIDL_LOCAL_APPDATA,
-					 NULL, SHGFP_TYPE_CURRENT, seedpath))) {
+					 NULL, SHGFP_TYPE_CURRENT, profile)) &&
-	    strcat(seedpath, "\\PUTTY.RND");
+            try_random_seed_and_free(dupcat(profile, "\\PUTTY.RND",
-	    if (try_random_seed(seedpath, action, &rethandle))
+                                            (const char *)NULL),
-		return rethandle;
+                                     action, &rethandle))
-	}
+            return rethandle;
 
 	if (SUCCEEDED(p_SHGetFolderPathA(NULL, CSIDL_APPDATA,
-					 NULL, SHGFP_TYPE_CURRENT, seedpath))) {
+					 NULL, SHGFP_TYPE_CURRENT, profile)) &&
-	    strcat(seedpath, "\\PUTTY.RND");
+            try_random_seed_and_free(dupcat(profile, "\\PUTTY.RND",
-	    if (try_random_seed(seedpath, action, &rethandle))
+                                            (const char *)NULL),
-		return rethandle;
+                                     action, &rethandle))
-	}
+            return rethandle;
     }
 
     /*
@@ -561,28 +568,36 @@ static HANDLE access_random_seed(int action)
      * user's home directory.
      */
     {
-	int len, ret;
+	char drv[MAX_PATH], path[MAX_PATH];
 
-	len =
+        DWORD drvlen = GetEnvironmentVariable("HOMEDRIVE", drv, sizeof(drv));
-	    GetEnvironmentVariable("HOMEDRIVE", seedpath,
+        DWORD pathlen = GetEnvironmentVariable("HOMEPATH", path, sizeof(path));
-				   sizeof(seedpath));
+
-	ret =
+        /* We permit %HOMEDRIVE% to expand to an empty string, but if
-	    GetEnvironmentVariable("HOMEPATH", seedpath + len,
+         * %HOMEPATH% does that, we abort the attempt. Same if either
-				   sizeof(seedpath) - len);
+         * variable overflows its buffer. */
-	if (ret != 0) {
+        if (drvlen == 0)
-	    strcat(seedpath, "\\PUTTY.RND");
+            drv[0] = '\0';
-	    if (try_random_seed(seedpath, action, &rethandle))
+
-		return rethandle;
+        if (drvlen < lenof(drv) && pathlen < lenof(path) && pathlen > 0 &&
-	}
+            try_random_seed_and_free(
+                dupcat(drv, path, "\\PUTTY.RND", (const char *)NULL),
+                action, &rethandle))
+            return rethandle;
     }
 
     /*
      * And finally, fall back to C:\WINDOWS.
      */
-    GetWindowsDirectory(seedpath, sizeof(seedpath));
+    {
-    strcat(seedpath, "\\PUTTY.RND");
+        char windir[MAX_PATH];
-    if (try_random_seed(seedpath, action, &rethandle))
+        DWORD len = GetWindowsDirectory(windir, sizeof(windir));
-	return rethandle;
+        if (len < lenof(windir) &&
+            try_random_seed_and_free(
+                dupcat(windir, "\\PUTTY.RND", (const char *)NULL),
+                action, &rethandle))
+            return rethandle;
+    }
 
     /*
      * If even that failed, give up.