* scandir.cc (scandir): Assume namelist is always valid, per POSIX.

	(CID 60021).
	* sec_auth.cc (cygwin_logon_user): Securely erase password copy.
	(lsaprivkeyauth): Avoid trying to dereference data if no key is stored
	in the registry (CID 60122).  Securely erase passwords after usage.

3 files changed, 48 insertions, 29 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 3ff4f896c..3e84f4e2e 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,11 @@
+2014-05-20  Corinna Vinschen  <corinna@vinschen.de>
+
+	* scandir.cc (scandir): Assume namelist is always valid, per POSIX.
+	(CID 60021).
+	* sec_auth.cc (cygwin_logon_user): Securely erase password copy.
+	(lsaprivkeyauth): Avoid trying to dereference data if no key is stored
+	in the registry (CID 60122).  Securely erase passwords after usage.
+
 2014-05-19  Corinna Vinschen  <corinna@vinschen.de>
 
 	* pseudo-reloc.cc (__report_error): Raise size of module name buffer to
diff --git a/winsup/cygwin/scandir.cc b/winsup/cygwin/scandir.cc
index 53d635f57..84a6b214b 100644
--- a/winsup/cygwin/scandir.cc
+++ b/winsup/cygwin/scandir.cc
@@ -1,6 +1,6 @@
 /* scandir.cc
 
-   Copyright 1998, 1999, 2000, 2001, 2003, 2005 Red Hat, Inc.
+   Copyright 1998, 1999, 2000, 2001, 2003, 2005, 2014 Red Hat, Inc.
 
    Written by Corinna Vinschen <corinna.vinschen@cityweb.de>
 
@@ -87,7 +87,6 @@ scandir (const char *dir,
   closedir (dirp);
 
   qsort (nl, count, sizeof *nl, (int (*)(const void *, const void *)) compar);
-  if (namelist)
-    *namelist = nl;
+  *namelist = nl;
   return count;
 }
diff --git a/winsup/cygwin/sec_auth.cc b/winsup/cygwin/sec_auth.cc
index 709874337..ac116ea22 100644
--- a/winsup/cygwin/sec_auth.cc
+++ b/winsup/cygwin/sec_auth.cc
@@ -166,6 +166,7 @@ cygwin_logon_user (const struct passwd *pw, const char *password)
       if (!hToken)
 	hToken = INVALID_HANDLE_VALUE;
     }
+  RtlSecureZeroMemory (passwd, NT_MAX_PATH);
   cygheap->user.reimpersonate ();
   debug_printf ("%R = logon_user(%s,...)", hToken, pw->pw_name);
   return hToken;
@@ -1185,8 +1186,9 @@ lsaprivkeyauth (struct passwd *pw)
   WCHAR user[UNLEN + 1];
   WCHAR key_name[MAX_DOMAIN_NAME_LEN + UNLEN + wcslen (SFU_LSA_KEY_SUFFIX) + 2];
   UNICODE_STRING key;
-  PUNICODE_STRING data;
+  PUNICODE_STRING data = NULL;
   cygsid psid;
+  BOOL ret;
 
   push_self_privilege (SE_TCB_PRIVILEGE, true);
 
@@ -1204,36 +1206,46 @@ lsaprivkeyauth (struct passwd *pw)
       RtlInitUnicodeString (&key, key_name);
       status = LsaRetrievePrivateData (lsa, &key, &data);
       if (!NT_SUCCESS (status))
-	{
-	  /* No Cygwin key, try Interix key. */
-	  if (!*domain)
-	    goto out;
-	  __small_swprintf (key_name, L"%W_%W%W",
-			    domain, user, SFU_LSA_KEY_SUFFIX);
-	  RtlInitUnicodeString (&key, key_name);
-	  status = LsaRetrievePrivateData (lsa, &key, &data);
-	  if (!NT_SUCCESS (status))
-	    goto out;
-	}
+	data = NULL;
     }
-
-  /* The key is not 0-terminated. */
-  PWCHAR passwd;
-  passwd = (PWCHAR) alloca (data->Length + sizeof (WCHAR));
-  *wcpncpy (passwd, data->Buffer, data->Length / sizeof (WCHAR)) = L'\0';
-  LsaFreeMemory (data);
-  debug_printf ("Try logon for %W\\%W", domain, user);
-  if (!LogonUserW (user, domain, passwd, LOGON32_LOGON_INTERACTIVE,
-		   LOGON32_PROVIDER_DEFAULT, &token))
+  /* No Cygwin key, try Interix key. */
+  if (!data && *domain)
     {
-      __seterrno ();
-      token = NULL;
+      __small_swprintf (key_name, L"%W_%W%W",
+			domain, user, SFU_LSA_KEY_SUFFIX);
+      RtlInitUnicodeString (&key, key_name);
+      status = LsaRetrievePrivateData (lsa, &key, &data);
+      if (!NT_SUCCESS (status))
+	data = NULL;
     }
-  else
-    token = get_full_privileged_inheritable_token (token);
+  /* Found an entry?  Try to logon. */
+  if (data)
+    {
+      /* The key is not 0-terminated. */
+      PWCHAR passwd;
+      size_t pwdsize = data->Length + sizeof (WCHAR);
+
+      passwd = (PWCHAR) alloca (pwdsize);
+      *wcpncpy (passwd, data->Buffer, data->Length / sizeof (WCHAR)) = L'\0';
+      /* Weird:  LsaFreeMemory invalidates the content of the UNICODE_STRING
+	 structure, but it does not invalidate the Buffer content. */
+      RtlSecureZeroMemory (data->Buffer, data->Length);
+      LsaFreeMemory (data);
+      debug_printf ("Try logon for %W\\%W", domain, user);
+      ret = LogonUserW (user, domain, passwd, LOGON32_LOGON_INTERACTIVE,
+			LOGON32_PROVIDER_DEFAULT, &token);
+      RtlSecureZeroMemory (passwd, pwdsize);
+      if (!ret)
+	{
+	  __seterrno ();
+	  token = NULL;
+	}
+      else
+	token = get_full_privileged_inheritable_token (token);
+    }
+  lsa_close_policy (lsa);
 
 out:
-  lsa_close_policy (lsa);
   pop_self_privilege ();
   return token;
 }