I noticed some issues in ReauthenticateUserWindows:

The doc for LsaConnectUntrusted says "The handle should be closed using the LsaDeregisterLogonProcess function." which we do, but then the ScopedHANDLE additionally calls CloseHandle which seems like one too many closings. lsa should probably get its own separate RAII struct that calls LsaDeregisterLogonProcess. (Also this would ensure that LsaDeregisterLogonProcess gets called even if we take one of the early returns)

Second, this pattern around LsaLogonUser is fragile:

  HANDLE token;
  NTSTATUS sts = LsaLogonUser(...);
  ScopedHANDLE scopedToken(token);
  if (sts ...) {

Generally the contract with Windows APIs is that you can rely on them to populate output parameters as long as the API succeeds.

If the API fails, token may be left uninitialized, and the RAII helper's CloseHandle could do bad things. Either the ScopedHandle should be established after checking the return value, or token should be initialized to a safe value before the API call.

The priority flag is not set for this bug.
:keeler, could you have a look please?

For more information, please visit auto_nag documentation.

https://hg.mozilla.org/mozilla-central/rev/433320ec2147