From 9c6a600e5bf02d86d2eec8fa47be1277cb22ed8f Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 27 Feb 2016 09:25:23 +0000
Subject: [PATCH] Make get_user_sid() return the cached copy if one already
 exists.

A user reported in January that locking down our process ACL causes
get_user_sid's call to OpenProcessToken to fail with a permissions
error. This _shouldn't_ be important, because we'll already have found
and cached the user SID before getting that far - but unfortunately
the call to get_user_sid in winnpc.c was bypassing the cache and
trying the whole process again.

This fix changes the memory ownership semantics of get_user_sid():
it's now an error to free the value it gives you, or else the *next*
call to get_user_sid() will return a stale pointer. Hence, also
removed those frees everywhere they appear.
---
 windows/winnpc.c   | 3 ---
 windows/winpgnt.c  | 4 ----
 windows/winpgntc.c | 1 -
 windows/winsecur.c | 5 ++++-
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/windows/winnpc.c b/windows/winnpc.c
index 0e8ac699..5927df32 100644
--- a/windows/winnpc.c
+++ b/windows/winnpc.c
@@ -79,7 +79,6 @@ Socket new_named_pipe_client(const char *pipename, Plug plug)
         ret = new_error_socket(err, plug);
         sfree(err);
         CloseHandle(pipehandle);
-        sfree(usersid);
         return ret;
     }
 
@@ -89,12 +88,10 @@ Socket new_named_pipe_client(const char *pipename, Plug plug)
         sfree(err);
         CloseHandle(pipehandle);
         LocalFree(psd);
-        sfree(usersid);
         return ret;
     }
 
     LocalFree(psd);
-    sfree(usersid);
 
     return make_handle_socket(pipehandle, pipehandle, plug, TRUE);
 }
diff --git a/windows/winpgnt.c b/windows/winpgnt.c
index f4194a68..f6371538 100644
--- a/windows/winpgnt.c
+++ b/windows/winpgnt.c
@@ -1934,7 +1934,6 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
 			debug(("couldn't get default SID\n"));
 #endif
                         CloseHandle(filemap);
-                        sfree(ourself);
 			return 0;
                     }
 
@@ -1947,7 +1946,6 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
                                rc));
 #endif
                         CloseHandle(filemap);
-                        sfree(ourself);
                         sfree(ourself2);
 			return 0;
 		    }
@@ -1968,7 +1966,6 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
                         !EqualSid(mapowner, ourself2)) {
                         CloseHandle(filemap);
                         LocalFree(psd);
-                        sfree(ourself);
                         sfree(ourself2);
 			return 0;      /* security ID mismatch! */
                     }
@@ -1976,7 +1973,6 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
 		    debug(("security stuff matched\n"));
 #endif
                     LocalFree(psd);
-                    sfree(ourself);
                     sfree(ourself2);
 		} else {
 #ifdef DEBUG_IPC
diff --git a/windows/winpgntc.c b/windows/winpgntc.c
index 036ca759..06649abc 100644
--- a/windows/winpgntc.c
+++ b/windows/winpgntc.c
@@ -182,6 +182,5 @@ int agent_query(void *in, int inlen, void **out, int *outlen,
     sfree(mapname);
     if (psd)
         LocalFree(psd);
-    sfree(usersid);
     return 1;
 }
diff --git a/windows/winsecur.c b/windows/winsecur.c
index 91ce7e92..95c1b6e1 100644
--- a/windows/winsecur.c
+++ b/windows/winsecur.c
@@ -44,6 +44,9 @@ PSID get_user_sid(void)
     DWORD toklen, sidlen;
     PSID sid = NULL, ret = NULL;
 
+    if (usersid)
+        return usersid;
+
     if (!got_advapi())
         goto cleanup;
 
@@ -73,7 +76,7 @@ PSID get_user_sid(void)
 
     /* Success. Move sid into the return value slot, and null it out
      * to stop the cleanup code freeing it. */
-    ret = sid;
+    ret = usersid = sid;
     sid = NULL;
 
   cleanup:
-- 
2.45.2