Confusing gotcha: PSECURITY_DESCRIPTOR is not a pointer to a SECURITY_DESCRIPTOR

Date:December 23, 2015 / year-entry #270
Tags:code
Orig Link:https://blogs.msdn.microsoft.com/oldnewthing/20151223-00/?p=92701
Comments:    12
Summary:It's just an untyped pointer.

There is a structure called a SECURITY_DESCRIPTOR. It describes the layout of an absolute security descriptor.

There is also a structure called a SECURITY_DESCRIPTOR_RELATIVE. It describes the layout of a relative security descriptor.

And then there is a type called PSECURITY_DESCRIPTOR. You might think based on its name that it is a pointer to a SECURITY_DESCRIPTOR. But it's not. It is defined as 

typedef PVOID PSECURITY_DESCRIPTOR;
// equivalent to
// typedef void *PSECURITY_DESCRIPTOR;

Most code that accept security descriptors don't care whether the security descriptor is absolute or relative. They just pass the security descriptor through to functions like Access­Check. And the name for a generic "pointer to some type of security descriptor, maybe relative, maybe absolute" is PSECURITY_DESCRIPTOR.

You rarely notice this switcheroo because code that deals with security descriptors typically use helper functions to do the heavy lifting. You notice this problem if you try to use something like std::unique_ptr to manage the lifetime of a security descriptor. 

template<typename T>
struct LocalAlloc_delete
{
 LocalAlloc_delete() { }
 void operator()(T* p) throw() { LocalFree(p); }
};

template<typename T>
using unique_localptr = std::unique_ptr<T, LocalAlloc_delete<T>>;

void some_function()
{
 PSECURITY_DESCRIPTOR result;
 ConvertStringSecurityDescriptorToSecurityDescriptorW(
  L"O:AOG:DAD:(A;;RPWPCCDCLCSWRCWDWOGA;;;S-1-0-0)",
  SDDL_REVISION_1, &result, nullptr);

 // compiler error here
 unique_localptr<SECURITY_DESCRIPTOR> sd(result);

 .. do stuff with sd ...
}

The compiler complains because result is a PSECURITY_DESCRIPTOR, but it expects a SECURITY_DESCRIPTOR*.

I can't think of a clean way out of this. Here are some ugly ways out: 

// ugly option 1 - cast it away
unique_localptr<SECURITY_DESCRIPTOR>
    sd(reinterpret_cast<SECURITY_DESCRIPTOR*>(result));

// ugly option 2 - special knowledge about PSECURITY_DESCRIPTOR
unique_localptr<void> sd(result);

// ugly option 3 - general, but an awful lot of typing
 unique_localptr<
    std::remove_pointer<PSECURITY_DESCRIPTOR>::type> sd(result);

In retrospect, the structure for an absolute security descriptor should have been named SECURITY_DESCRIPTOR_ABSOLUTE. My guess is that the name is historical: Initially, the only kind of security descriptor was absolute. Later, relative security descriptors were invented, and the easiest way to retrofit them into the existing interfaces was to make PSECURITY_DESCRIPTOR the generic security descriptor pointer.

Comments (12)
  1. Medinoc says:
    December 23, 2015 at 8:02 am

    I guess that’s the problem with C being the primary language of the Windows API, because in C++ this would probably have been solved with a base class and two derived classes…

    //Nitpicker’s corner
    Why use a reinterpret_cast from void*, where a static_cast would suffice?

    1. alegr1 says:
      December 23, 2015 at 9:59 am

      You know that Raymond is a fan of reinterpret_cast, don’t you? It’s like more brutal and dangerous variant of C-cast, while you can pretend you enjoy type safety of C++.

      1. Raymond Chen - MSFT says:
        December 23, 2015 at 10:04 am

        I use static_cast when I’m converting between related pointers. But void* isn’t related to anything, so I use reinterpret_cast to emphasize “This is scary, you’d better be right because the compiler isn’t going to help.”

        1. Darran Rowe says:
          December 23, 2015 at 1:24 pm

          I always end up seeing it in the opposite way. Because of that implicit conversion to void *, it is actually related to everything.
          Of course, the result is the same. You are going to have to be really sure when you cast it back, otherwise bad things are going to happen.
          I was just mentioning this because it was interesting how a differing point of view ended up with the same result.

          1. Raymond Chen - MSFT says:
            December 23, 2015 at 1:32 pm

            I tend to use static_cast for “The compiler will double-check me here” and reinterpret_cast for “Pay close attention here.” Although you can technically use static_cast with void*, I think it’s misleading and lets you hide a dangerous cast inside something that looks safe.

        2. Alex Cohn says:
          December 23, 2015 at 11:28 pm

          It’s a pity that there is no compiler flag to mark all casts from void* (including typedef’ed) as dangerous. I’d also follow such cast with some assert() wherever possible.

  2. praetorian says:
    December 23, 2015 at 12:51 pm

    There is another option. unique_ptr has a nifty feature where the managed pointer type can be switched from T* to Deleter::pointer if that type exists. So just provide a specialization of LocalAlloc_delete for SECURITY_DESCRIPTOR

    template
    struct LocalAlloc_delete
    {
    void operator()(PSECURITY_DESCRIPTOR p) const throw( )
    {
    LocalFree(p);
    }
    using pointer = PSECURITY_DESCRIPTOR;
    };

    Now unique_localptr will happily accept a PSECURITY_DESCRIPTOR

    http://rextester.com/UOC70003

    1. praetorian says:
      December 23, 2015 at 12:53 pm

      Obviously I have no clue how to do code formatting in comments.

      1. Raymond Chen - MSFT says:
        December 23, 2015 at 1:31 pm

        I don’t think you can, so don’t worry. (P.S., cool trick.)

  3. 640k says:
    December 24, 2015 at 1:13 am

    No problem, just change it to what’s expected.

  4. Neil says:
    December 24, 2015 at 2:30 am

    Instead of:

    template using unique_localptr = std::unique_ptr<T, LocalAlloc_delete>;
    …
    unique_localptr sd(result);

    why not

    template
    std::unique_ptr<T, LocalAlloc_delete> unique_localptr(T* raw_ptr)
    {
    return std::unique_ptr<T, LocalAlloc_delete>(raw_ptr);
    }
    …
    auto sd(unique_localptr(result));

  5. French Guy says:
    December 24, 2015 at 7:18 am

    Wouldn’t it have been better to name the generic security descriptor SECURITY_DESCRIPTOR_GENERIC and create a SECURITY_DESCRIPTOR_ABSOLUTE alias for SECURITY_DESCRIPTOR (keeping the old name around for compatibility)?

Comments are closed.


*DISCLAIMER: I DO NOT OWN THIS CONTENT. If you are the owner and would like it removed, please contact me. The content herein is an archived reproduction of entries from Raymond Chen's "Old New Thing" Blog (most recent link is here). It may have slight formatting modifications for consistency and to improve readability.

WHY DID I DUPLICATE THIS CONTENT HERE? Let me first say this site has never had anything to sell and has never shown ads of any kind. I have nothing monetarily to gain by duplicating content here. Because I had made my own local copy of this content throughout the years, for ease of using tools like grep, I decided to put it online after I discovered some of the original content previously and publicly available, had disappeared approximately early to mid 2019. At the same time, I present the content in an easily accessible theme-agnostic way.

The information provided by Raymond's blog is, for all practical purposes, more authoritative on Windows Development than Microsoft's own MSDN documentation and should be considered supplemental reading to that documentation. The wealth of missing details provided by this blog that Microsoft could not or did not document about Windows over the years is vital enough, many would agree an online "backup" of these details is a necessary endeavor. Specifics include:

<-- Back to Old New Thing Archive Index