14

ReSharper warns me about a possible NullReferenceException in

WindowsIdentity windowsIdentity = new WindowsIdentity(WindowsIdentity.GetCurrent().Token);

I looked in MSDN doc but didn't see any mention of this. Also, it doesn't make sense since if you run an executable, you have to be logged on.

Is this just a ReSharper search pattern?

Pang
  • 9,564
  • 146
  • 81
  • 122
Noich
  • 14,631
  • 15
  • 62
  • 90

4 Answers4

21

Using ILSpy, you can look at a decompiled version of GetCurrent and GetCurrentInternal, which GetCurrent calls.

GetCurrent:

public static WindowsIdentity GetCurrent()
{
    return WindowsIdentity.GetCurrentInternal(TokenAccessLevels.MaximumAllowed, false);
}

GetCurrentInternal:

internal static WindowsIdentity GetCurrentInternal(TokenAccessLevels desiredAccess, bool threadOnly)
{
    int errorCode = 0;
    bool flag;
    SafeTokenHandle currentToken = WindowsIdentity.GetCurrentToken(desiredAccess, threadOnly, out flag, out errorCode);
    if (currentToken != null && !currentToken.IsInvalid)
    {
        WindowsIdentity windowsIdentity = new WindowsIdentity();
        windowsIdentity.m_safeTokenHandle.Dispose();
        windowsIdentity.m_safeTokenHandle = currentToken;
        return windowsIdentity;
    }
    if (threadOnly && !flag)
    {
        return null;
    }
    throw new SecurityException(Win32Native.GetMessage(errorCode));
}

Since threadOnly is always false when calling from GetCurrent, and the currentToken must be valid for the other return statement, I don't think you're at risk of getting a null WindowsIdentity.

Pang
  • 9,564
  • 146
  • 81
  • 122
nitzanms
  • 1,786
  • 12
  • 35
6

ReSharper should handle this.

In directory <ReSharper install dir>\v7.1\Bin\ExternalAnnotations\.NETFramework\mscorlib, the external annotations file Nullness.Manual.xml defines the following annotations:

<!-- RSRP-328266 -->
<member name="M:System.Security.Principal.WindowsIdentity.GetCurrent">
  <attribute ctor="M:JetBrains.Annotations.NotNullAttribute.#ctor" />
</member>
<member name="M:System.Security.Principal.WindowsIdentity.GetCurrent(System.Boolean)">
  <attribute ctor="M:JetBrains.Annotations.ContractAnnotationAttribute.#ctor(System.String)">
    <argument>false=&gt;notnull</argument>
  </attribute>
</member>
<member name="M:System.Security.Principal.WindowsIdentity.GetCurrent(System.Security.Principal.TokenAccessLevels)">
  <attribute ctor="M:JetBrains.Annotations.NotNullAttribute.#ctor" />
</member>

However, I am also getting the warning about a possible NullReferenceException on WindowsIdentity.GetCurrent(). For some reason, ReSharper isn't recognizing its own external annotation attributes. If this is a known bug, or if there is a fix for this problem, please reply.

Pang
  • 9,564
  • 146
  • 81
  • 122
John Beyer
  • 673
  • 7
  • 8
  • 2
    well, I googled it for you :) This seems to be our little friend: http://youtrack.jetbrains.com/issue/RSRP-328266 – Noich Apr 24 '13 at 06:28
  • Exactly. The above annotations are supposed to fix 328266 (hence the comment on the first line of the XML snippet), but for whatever reason, the fix doesn't seem to be working. If this indicates a problem in my R# settings or configuration, please elaborate. – John Beyer Apr 29 '13 at 18:38
  • I really have no idea :) You'll have to take it with their QA. – Noich Apr 30 '13 at 05:33
2

It sounds like a false report from ReSharper.

The MSDN page for GetCurrent makes no mention of returning null in any circumstances.

As you point out, there has to be a current user (of one kind or another), so this should always return a valid object - if you have permissions.

It can raise a SecurityException, but that's a different error and your code would fail anyway. If this is a possibility, then you might want to rearrange your code:

WindowsIdentity currentIdentity = null;
try
{
    currentIdentity = WindowsIdentity.GetCurrent();
    // Carry on if there's nothing you can do
    WindowsIdentity newIdentity = new WindowsIdentity(currentIdentity.Token);
}
catch (SecurityException ex)
{
    // Do something, logging, display error etc.
}
Pang
  • 9,564
  • 146
  • 81
  • 122
ChrisF
  • 134,786
  • 31
  • 255
  • 325
1

According to the disassembly, null could be returned.

See: GetCurrentInternal(TokenAccessLevels desiredAccess, bool threadOnly)

Disclaimer: I am too lazy to dissect the specific condition :)

leppie
  • 115,091
  • 17
  • 196
  • 297