#include <windows.h> #include <aclapi.h> #include <tchar.h> int main() { PACL pDACL = NULL; PSECURITY_DESCRIPTOR pSD = NULL; ::GetNamedSecurityInfo(_T("ObjectName"), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD); auto test = pDACL == NULL; // V547 Expression 'pDACL == 0' is always true. return 0; }

ppDacl



A pointer to a variable that receives a pointer to the DACL in the returned security descriptor or NULL if the security descriptor has no DACL. The returned pointer is valid only if you set the DACL_SECURITY_INFORMATION flag. Also, this parameter can be NULL if you do not need the DACL.



Our team provides quick and effective customer support. User requests are handled solely by programmers since our clients are programmers themselves and they often ask tricky questions. Today I'm going to tell you about a recent request concerning one false positive that even forced me to carry out a small investigation to solve the problem.We work hard to reduce the number of false positives generated by PVS-Studio to a minimum. Unfortunately, static analyzers are often unable to tell correct code from a bug because they just don't have enough information. False positives are, therefore, inevitable. However, it's not a problem since you can easily customize the analyzer so that 9 out of 10 warnings will point to genuine bugs.While false positives might not seem a big deal, we never stop fighting them by improving our diagnostics. Some blatant false positives are caught by our team; others are reported by our clients and free-version users.One of our clients recently sent us an email reading something like this:It's not hard to guess how our users see false positives like that. Thefunction obviously modifies the value of the variable. What prevented the developers from making a handler for simple cases like that? And why isn't the warning issued in every session? Maybe it's a bug in the analyzer itself, say, an uninitialized variable?Alas… Supporting users of a static code analyzer is not an easy job, but it was my own choice to do that. So, I rolled up my sleeves and got down to investigating the problem.I started off by checking the description of the GetNamedSecurityInfo function and making sure that its call indeed implied modifying the value of thevariable. Here's the description of the 6th argument:I know that PVS-Studio should obviously be able to handle such simple code without generating a false warning. At that point, my intuition was already telling me that the case wasn't a trivial one and that it was going to take quite a while to solve.My misgivings were confirmed when I failed to reproduce the false positive either with our current alpha version of the analyzer or the version installed on the user's computer. No matter what I did, the analyzer kept silent.I asked the client to send me the preprocessed i-file generated for the example program. He did that, and I went on with my investigation.The analyzer did produce the false positive on that file right away. On the one hand, it was good that I had finally managed to reproduce it. On the other hand, I had a feeling that could be best illustrated by this picture:

::GetNamedSecurityInfoW(L"ObjectName", SE_FILE_OBJECT, (0x00000004L), 0, 0, &pDACL, 0, &pSD);

__declspec(dllimport) DWORD __stdcall GetNamedSecurityInfoW( LPCWSTR pObjectName, SE_OBJECT_TYPE ObjectType, SECURITY_INFORMATION SecurityInfo, PSID * ppsidOwner, PSID * ppsidGroup, PACL * ppDacl, PACL * ppSacl, PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

Why this feeling? You see, I know perfectly well how both the analyzer and the V547 diagnostic work. There's simply no way they could generate a false positive like that, ever!OK, let's make some tea and continue.The call to thefunction expands into the following code:This code looks the same both in the i-file preprocessed on my computer and the file sent by the user.Hmm… OK, let's look at the declaration of this function. Here's what I've got in my file:All is logical and clear. Nothing unusual.Then I peek into the user's file and…

__declspec(dllimport) DWORD __stdcall GetNamedSecurityInfoW( LPCWSTR pObjectName, SE_OBJECT_TYPE ObjectType, SECURITY_INFORMATION SecurityInfo, const PSID * ppsidOwner, const PSID * ppsidGroup, const PACL * ppDacl, const PACL * ppSacl, PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

PACL pDACL = NULL; PSECURITY_DESCRIPTOR pSD = NULL; ::GetNamedSecurityInfo(_T("ObjectName"), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD); auto test = pDACL == NULL; // V547 Expression 'pDACL == 0' is always true.

WINADVAPI DWORD WINAPI GetNamedSecurityInfoW( __in LPWSTR pObjectName, __in SE_OBJECT_TYPE ObjectType, __in SECURITY_INFORMATION SecurityInfo, __out_opt PSID * ppsidOwner, __out_opt PSID * ppsidGroup, __out_opt PACL * ppDacl, __out_opt PACL * ppSacl, __out_opt PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

WINADVAPI DWORD WINAPI GetNamedSecurityInfoW( __in LPCWSTR pObjectName, __in SE_OBJECT_TYPE ObjectType, __in SECURITY_INFORMATION SecurityInfo, __out_opt const PSID * ppsidOwner, __out_opt const PSID * ppsidGroup, __out_opt const PACL * ppDacl, __out_opt const PACL * ppSacl, __out PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

What I see there doesn't belong to our reality:Note that the formal parameteris marked asWhat's that!? What is it doing here!?Well, at least I know for sure that the analyzer is innocent and I can defend its honor.The argument is a pointer to a constant object. It turns out that, from the analyzer's point of view, thefunction can't modify the object referred to by the pointer. Therefore, in the following code:thevariable can't change, which the analyzer rightly warns us about (Expression 'pDACL == 0' is always true.).OK, now we know what triggers the warning. What we still don't know is where thatkeyword came from. It just can't be there!Well, I have one guess, and it is confirmed by what I find on the Internet. It turns out that there's an old version of the file aclapi.h with an incorrect function description. I have also run across a couple of interesting links:So, once upon a time, there was a function description in the aclapi.h file (6.0.6002.18005-Windows 6.0):Then someone changed the type of theparameter but messed up the types of the pointers along the way by adding thekeyword. And the aclapi.h file (6.1.7601.23418-Windows 7.0) ended up as follows:

WINADVAPI DWORD WINAPI GetNamedSecurityInfoW( _In_ LPCWSTR pObjectName, _In_ SE_OBJECT_TYPE ObjectType, _In_ SECURITY_INFORMATION SecurityInfo, _Out_opt_ PSID * ppsidOwner, _Out_opt_ PSID * ppsidGroup, _Out_opt_ PACL * ppDacl, _Out_opt_ PACL * ppSacl, _Out_ PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

Now it was clear that our user had been working with that very incorrect version of aclapi.h, which he then confirmed in his email. I couldn't reproduce the bug because I was using a more recent version.This is what the fixed function description looks like in the latest aclapi.h file (6.3.9600.17415-Windows_8.1).

The type of theargument is still the same, but the extra's are gone. All is good again, but there are still broken headers in use somewhere out there.I explain all of that to the customer, and he is happy to see the problem solved. Moreover, he has found out why the false positive didn't occur regularly:That's all. The investigation is over. We discuss the issue with the client and decide not to add any kludge to the analyzer to make it able to handle this header-file bug. The most important thing is that we have figured out the problem. «Case dismissed», as they say.PVS-Studio is a complex software product, which gathers large amounts of information from programs' code and utilizes it in various analysis techniques . In this particular case, it turned out to be too smart, ending up with a false positive because of an incorrect function description.