Another big batch of memory leak fixes, again mostly on error paths.

The most interesting one is printer_add_enum, which I've modified to
take a char ** rather than a char * so that it can both realloc its
input buffer _and_ return NULL to indicate error.

git-svn-id: svn://svn.tartarus.org/sgt/putty@9959 cda61777-01e9-0310-a592-d414129be87e

diff --git a/windows/winctrls.c b/windows/winctrls.c
index 60e32f5..49c13e8 100644 (file)
--- a/windows/winctrls.c
+++ b/windows/winctrls.c
@@ -448,6 +448,8 @@ char *staticwrap(struct ctlpos *cp, HWND hwnd, char *text, int *lines)
 
     if (lines) *lines = nlines;
 
+    sfree(pwidths);
+
     return ret;
 }
 
@@ -1665,7 +1667,9 @@ void winctrl_layout(struct dlgparam *dp, struct winctrls *wc,
            winctrl_add_shortcuts(dp, c);
            if (actual_base_id == base_id)
                base_id += num_ids;
-       }
+       } else {
+            sfree(data);
+        }
 
        if (colstart >= 0) {
            /*

diff --git a/windows/window.c b/windows/window.c
index b2a5c5c..b477d6a 100644 (file)
--- a/windows/window.c
+++ b/windows/window.c
@@ -2197,8 +2197,10 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
                reconfig_result =
                    do_reconfig(hwnd, back ? back->cfg_info(backhandle) : 0);
                reconfiguring = FALSE;
-               if (!reconfig_result)
+               if (!reconfig_result) {
+                    conf_free(prev_conf);
                    break;
+                }
 
                conf_cache_data();
 
@@ -4903,10 +4905,17 @@ void write_clip(void *frontend, wchar_t * data, int *attr, int len, int must_des
            GlobalFree(clipdata2);
        return;
     }
-    if (!(lock = GlobalLock(clipdata)))
+    if (!(lock = GlobalLock(clipdata))) {
+        GlobalFree(clipdata);
+        GlobalFree(clipdata2);
        return;
-    if (!(lock2 = GlobalLock(clipdata2)))
+    }
+    if (!(lock2 = GlobalLock(clipdata2))) {
+        GlobalUnlock(clipdata);
+        GlobalFree(clipdata);
+        GlobalFree(clipdata2);
        return;
+    }
 
     memcpy(lock, data, len * sizeof(wchar_t));
     WideCharToMultiByte(CP_ACP, 0, data, len, lock2, len2, NULL, NULL);

diff --git a/windows/winjump.c b/windows/winjump.c
index 563d42c..abefe44 100644 (file)
--- a/windows/winjump.c
+++ b/windows/winjump.c
@@ -410,16 +410,20 @@ static IShellLink *make_shell_link(const char *appname,
     /* Check if this is a valid session, otherwise don't add. */
     if (sessionname) {
         psettings_tmp = open_settings_r(sessionname);
-        if (!psettings_tmp)
+        if (!psettings_tmp) {
+            sfree(app_path);
             return NULL;
+        }
         close_settings_r(psettings_tmp);
     }
 
     /* Create the new item. */
     if (!SUCCEEDED(CoCreateInstance(&CLSID_ShellLink, NULL,
                                     CLSCTX_INPROC_SERVER,
-                                    COMPTR(IShellLink, &ret))))
+                                    COMPTR(IShellLink, &ret)))) {
+        sfree(app_path);
         return NULL;
+    }
 
     /* Set path, parameters, icon and description. */
     ret->lpVtbl->SetPath(ret, app_path);

diff --git a/windows/winpgen.c b/windows/winpgen.c
index aeebb62..9fa35af 100644 (file)
--- a/windows/winpgen.c
+++ b/windows/winpgen.c
@@ -420,11 +420,11 @@ static int save_ssh1_pubkey(char *filename, struct RSAKey *key)
     char *dec1, *dec2;
     FILE *fp;
 
-    dec1 = bignum_decimal(key->exponent);
-    dec2 = bignum_decimal(key->modulus);
     fp = fopen(filename, "wb");
     if (!fp)
        return 0;
+    dec1 = bignum_decimal(key->exponent);
+    dec2 = bignum_decimal(key->modulus);
     fprintf(fp, "%d %s %s %s\n",
            bignum_bitcount(key->modulus), dec1, dec2, key->comment);
     fclose(fp);
@@ -1256,9 +1256,11 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
            if (!state->generation_thread_exists) {
                char filename[FILENAME_MAX];
                if (prompt_keyfile(hwnd, "Load private key:",
-                                  filename, 0, LOWORD(wParam)==IDC_LOAD))
-                   load_key_file(hwnd, state, filename_from_str(filename),
-                                 LOWORD(wParam) != IDC_LOAD);
+                                  filename, 0, LOWORD(wParam)==IDC_LOAD)) {
+                    Filename *fn = filename_from_str(filename);
+                   load_key_file(hwnd, state, fn, LOWORD(wParam) != IDC_LOAD);
+                    filename_free(fn);
+                }
            }
            break;
        }

diff --git a/windows/winpgnt.c b/windows/winpgnt.c
index e220d6b..1f6484f 100644 (file)
--- a/windows/winpgnt.c
+++ b/windows/winpgnt.c
@@ -811,8 +811,10 @@ static void *get_keylist1(int *length)
        retval = agent_query(request, 5, &vresponse, &resplen, NULL, NULL);
        assert(retval == 1);
        response = vresponse;
-       if (resplen < 5 || response[4] != SSH1_AGENT_RSA_IDENTITIES_ANSWER)
+       if (resplen < 5 || response[4] != SSH1_AGENT_RSA_IDENTITIES_ANSWER) {
+            sfree(response);
            return NULL;
+        }
 
        ret = snewn(resplen-5, unsigned char);
        memcpy(ret, response+5, resplen-5);
@@ -846,8 +848,10 @@ static void *get_keylist2(int *length)
        retval = agent_query(request, 5, &vresponse, &resplen, NULL, NULL);
        assert(retval == 1);
        response = vresponse;
-       if (resplen < 5 || response[4] != SSH2_AGENT_IDENTITIES_ANSWER)
+       if (resplen < 5 || response[4] != SSH2_AGENT_IDENTITIES_ANSWER) {
+            sfree(response);
            return NULL;
+        }
 
        ret = snewn(resplen-5, unsigned char);
        memcpy(ret, response+5, resplen-5);
@@ -942,12 +946,17 @@ static void answer_msg(void *msg)
                goto failure;
            p += i;
            i = ssh1_read_bignum(p, msgend - p, &reqkey.modulus);
-           if (i < 0)
+           if (i < 0) {
+                freebn(reqkey.exponent);
                goto failure;
+            }
            p += i;
            i = ssh1_read_bignum(p, msgend - p, &challenge);
-           if (i < 0)
+           if (i < 0) {
+                freebn(reqkey.exponent);
+                freebn(reqkey.modulus);
                goto failure;
+            }
            p += i;
            if (msgend < p+16) {
                freebn(reqkey.exponent);
@@ -1437,10 +1446,12 @@ static void prompt_add_keyfile(void)
     of.lpstrTitle = "Select Private Key File";
     of.Flags = OFN_ALLOWMULTISELECT | OFN_EXPLORER;
     if (request_file(keypath, &of, TRUE, FALSE)) {
-       if(strlen(filelist) > of.nFileOffset)
+       if(strlen(filelist) > of.nFileOffset) {
            /* Only one filename returned? */
-           add_keyfile(filename_from_str(filelist));
-       else {
+            Filename *fn = filename_from_str(filelist);
+           add_keyfile(fn);
+            filename_free(fn);
+        } else {
            /* we are returned a bunch of strings, end to
             * end. first string is the directory, the
             * rest the filenames. terminated with an
@@ -1450,7 +1461,9 @@ static void prompt_add_keyfile(void)
            char *filewalker = filelist + strlen(dir) + 1;
            while (*filewalker != '\0') {
                char *filename = dupcat(dir, "\\", filewalker, NULL);
-               add_keyfile(filename_from_str(filename));
+                Filename *fn = filename_from_str(filename);
+               add_keyfile(fn);
+                filename_free(fn);
                sfree(filename);
                filewalker += strlen(filewalker) + 1;
            }
@@ -1908,6 +1921,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
 #ifdef DEBUG_IPC
                        debug(("couldn't get user SID\n"));
 #endif
+                        CloseHandle(filemap);
                        return 0;
                     }
 
@@ -1915,6 +1929,8 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
 #ifdef DEBUG_IPC
                        debug(("couldn't get default SID\n"));
 #endif
+                        CloseHandle(filemap);
+                        sfree(ourself);
                        return 0;
                     }
 
@@ -1926,6 +1942,9 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
                        debug(("couldn't get owner info for filemap: %d\n",
                                rc));
 #endif
+                        CloseHandle(filemap);
+                        sfree(ourself);
+                        sfree(ourself2);
                        return 0;
                    }
 #ifdef DEBUG_IPC
@@ -1944,6 +1963,9 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
                    if (!EqualSid(mapowner, ourself) &&
                         !EqualSid(mapowner, ourself2)) {
                         CloseHandle(filemap);
+                        LocalFree(psd);
+                        sfree(ourself);
+                        sfree(ourself2);
                        return 0;      /* security ID mismatch! */
                     }
 #ifdef DEBUG_IPC
@@ -2127,7 +2149,9 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show)
                command = "";
            break;
        } else {
-           add_keyfile(filename_from_str(argv[i]));
+            Filename *fn = filename_from_str(argv[i]);
+           add_keyfile(fn);
+            filename_free(fn);
            added_keys = TRUE;
        }
     }

diff --git a/windows/winpgntc.c b/windows/winpgntc.c
index 4074a16..7b8b509 100644 (file)
--- a/windows/winpgntc.c
+++ b/windows/winpgntc.c
@@ -209,8 +209,10 @@ int agent_query(void *in, int inlen, void **out, int *outlen,
 
     filemap = CreateFileMapping(INVALID_HANDLE_VALUE, psa, PAGE_READWRITE,
                                0, AGENT_MAX_MSGLEN, mapname);
-    if (filemap == NULL || filemap == INVALID_HANDLE_VALUE)
+    if (filemap == NULL || filemap == INVALID_HANDLE_VALUE) {
+        sfree(mapname);
        return 1;                      /* *out == NULL, so failure */
+    }
     p = MapViewOfFile(filemap, FILE_MAP_WRITE, 0, 0, 0);
     memcpy(p, in, inlen);
     cds.dwData = AGENT_COPYDATA_ID;
@@ -237,6 +239,7 @@ int agent_query(void *in, int inlen, void **out, int *outlen,
        data->hwnd = hwnd;
        if (CreateThread(NULL, 0, agent_query_thread, data, 0, &threadid))
            return 0;
+       sfree(mapname);
        sfree(data);
     }
 #endif
@@ -258,6 +261,7 @@ int agent_query(void *in, int inlen, void **out, int *outlen,
     }
     UnmapViewOfFile(p);
     CloseHandle(filemap);
+    sfree(mapname);
     if (psd)
         LocalFree(psd);
     sfree(usersid);

diff --git a/windows/winprint.c b/windows/winprint.c
index 1548c3c..bc5a5d1 100644 (file)
--- a/windows/winprint.c
+++ b/windows/winprint.c
@@ -18,39 +18,39 @@ struct printer_job_tag {
     HANDLE hprinter;
 };
 
-static char *printer_add_enum(int param, DWORD level, char *buffer,
-                              int offset, int *nprinters_ptr)
+static int printer_add_enum(int param, DWORD level, char **buffer,
+                            int offset, int *nprinters_ptr)
 {
     DWORD needed = 0, nprinters = 0;
 
-    buffer = sresize(buffer, offset+512, char);
+    *buffer = sresize(*buffer, offset+512, char);
 
     /*
      * Exploratory call to EnumPrinters to determine how much space
      * we'll need for the output. Discard the return value since it
      * will almost certainly be a failure due to lack of space.
      */
-    EnumPrinters(param, NULL, level, buffer+offset, 512,
+    EnumPrinters(param, NULL, level, (*buffer)+offset, 512,
                 &needed, &nprinters);
 
     if (needed < 512)
         needed = 512;
 
-    buffer = sresize(buffer, offset+needed, char);
+    *buffer = sresize(*buffer, offset+needed, char);
 
-    if (EnumPrinters(param, NULL, level, buffer+offset,
+    if (EnumPrinters(param, NULL, level, (*buffer)+offset,
                      needed, &needed, &nprinters) == 0)
-        return NULL;
+        return FALSE;
 
     *nprinters_ptr += nprinters;
 
-    return buffer;
+    return TRUE;
 }
 
 printer_enum *printer_start_enum(int *nprinters_ptr)
 {
     printer_enum *ret = snew(printer_enum);
-    char *buffer = NULL, *retval;
+    char *buffer = NULL;
 
     *nprinters_ptr = 0;                       /* default return value */
     buffer = snewn(512, char);
@@ -71,12 +71,9 @@ printer_enum *printer_start_enum(int *nprinters_ptr)
        ret->enum_level = 4;
     }
 
-    retval = printer_add_enum(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS,
-                             ret->enum_level, buffer, 0, nprinters_ptr);
-    if (!retval)
+    if (!printer_add_enum(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS,
+                          ret->enum_level, &buffer, 0, nprinters_ptr))
         goto error;
-    else
-        buffer = retval;
 
     switch (ret->enum_level) {
       case 4:

diff --git a/windows/winproxy.c b/windows/winproxy.c
index 75a7852..7bbd18f 100644 (file)
--- a/windows/winproxy.c
+++ b/windows/winproxy.c
@@ -180,6 +180,7 @@ Socket platform_new_connection(SockAddr addr, char *hostname,
     sa.bInheritHandle = TRUE;
     if (!CreatePipe(&us_from_cmd, &cmd_to_us, &sa, 0)) {
        ret->error = dupprintf("Unable to create pipes for proxy command");
+        sfree(cmd);
        return (Socket)ret;
     }
 
@@ -187,6 +188,7 @@ Socket platform_new_connection(SockAddr addr, char *hostname,
        CloseHandle(us_from_cmd);
        CloseHandle(cmd_to_us);
        ret->error = dupprintf("Unable to create pipes for proxy command");
+        sfree(cmd);
        return (Socket)ret;
     }
 

diff --git a/windows/winstore.c b/windows/winstore.c
index 66c7c75..47fef67 100644 (file)
--- a/windows/winstore.c
+++ b/windows/winstore.c
@@ -167,7 +167,10 @@ char *read_setting_s(void *handle, const char *key)
     ret = snewn(size+1, char);
     if (RegQueryValueEx((HKEY) handle, key, 0,
                        &type, ret, &size) != ERROR_SUCCESS ||
-       type != REG_SZ) return NULL;
+       type != REG_SZ) {
+        sfree(ret);
+        return NULL;
+    }
 
     return ret;
 }
@@ -190,6 +193,7 @@ FontSpec *read_setting_fontspec(void *handle, const char *name)
 {
     char *settingname;
     char *fontname;
+    FontSpec *ret;
     int isbold, height, charset;
 
     fontname = read_setting_s(handle, name);
@@ -199,19 +203,30 @@ FontSpec *read_setting_fontspec(void *handle, const char *name)
     settingname = dupcat(name, "IsBold", NULL);
     isbold = read_setting_i(handle, settingname, -1);
     sfree(settingname);
-    if (isbold == -1) return NULL;
+    if (isbold == -1) {
+        sfree(fontname);
+        return NULL;
+    }
 
     settingname = dupcat(name, "CharSet", NULL);
     charset = read_setting_i(handle, settingname, -1);
     sfree(settingname);
-    if (charset == -1) return NULL;
+    if (charset == -1) {
+        sfree(fontname);
+        return NULL;
+    }
 
     settingname = dupcat(name, "Height", NULL);
     height = read_setting_i(handle, settingname, INT_MIN);
     sfree(settingname);
-    if (height == INT_MIN) return NULL;
+    if (height == INT_MIN) {
+        sfree(fontname);
+        return NULL;
+    }
 
-    return fontspec_new(fontname, isbold, height, charset);
+    ret = fontspec_new(fontname, isbold, height, charset);
+    sfree(fontname);
+    return ret;
 }
 
 void write_setting_fontspec(void *handle, const char *name, FontSpec *font)
@@ -340,16 +355,18 @@ int verify_host_key(const char *hostname, int port,
      * Now read a saved key in from the registry and see what it
      * says.
      */
-    otherstr = snewn(len, char);
     regname = snewn(3 * (strlen(hostname) + strlen(keytype)) + 15, char);
 
     hostkey_regname(regname, hostname, port, keytype);
 
     if (RegOpenKey(HKEY_CURRENT_USER, PUTTY_REG_POS "\\SshHostKeys",
-                  &rkey) != ERROR_SUCCESS)
+                  &rkey) != ERROR_SUCCESS) {
+        sfree(regname);
        return 1;                      /* key does not exist in registry */
+    }
 
     readlen = len;
+    otherstr = snewn(len, char);
     ret = RegQueryValueEx(rkey, regname, NULL, &type, otherstr, &readlen);
 
     if (ret != ERROR_SUCCESS && ret != ERROR_MORE_DATA &&
@@ -412,6 +429,8 @@ int verify_host_key(const char *hostname, int port,
                RegSetValueEx(rkey, regname, 0, REG_SZ, otherstr,
                              strlen(otherstr) + 1);
        }
+
+        sfree(oldstyle);
     }
 
     RegCloseKey(rkey);