PWSTR Comparision fails!

Hi there,

Please excuse me if I am so silly asking in this question, I am a Java guy and newbie to this CPP stuff.

I am facing some problem when I do compare two PWSTR variables, please see the code:

Sample Output received with below code:
pszValidUser=TESTUSER1
pCred->pszUserName=TESTUSER1

even if both are same, validation says it's not equal...

Types defined:
pCred->userName: PWSTR_WLX_CLIENT_CREDENTIALS_INFO::pszUserName
PWSTR pszValidUser = (PWSTR)(LocalLock(LocalAlloc(LHND,userName.Length + 2)));
memcpy(pszValidUser,userName.Buffer, userName.Length);
pszValidUser[ (userName.Length / 2) ] = 0;
 
SysMessage(Event,"SUCCESS","pszValidUser:%S, with old user:%S",                             pszValidUser, pCred->pszUserName);
 
if (wcsicmp(pszValidUser, pCred->pszUserName)== 0)
        {
             SysMessage(Event,"SUCCESS","Match user %S\\%S with %S  at Client %s .",
                                pCred->pszDomain,
                                pCred->pszUserName,
                                pszValidUser,
                                szClientAddress);             
        }
        else
        {
           SysMessage(FatalError,"EXCEPTION","Client %s mismatch for user %S\\%S with %S.",
                                szClientAddress,
                                pCred->pszDomain,
                                pCred->pszUserName,
                                pszValidUser);
		    return FALSE;
        }

Open in new window

yarabatiAsked:
Who is Participating?

Improve company productivity with a Business Account.Sign Up

x
 
itsmeandnobodyelseConnect With a Mentor Commented:
>>>> here what I am doing is Iam taking the new user name to another variable pszValidUser and checking with it's old value before do overriding

Yes I know. My sample was wrong it should have been

    if (userName == pCred->pszUserName)
    {

If userName is an object of a (wide char) string class it has an operator==  defined which takes a const wchar_t* as argument. Then it also takes a PWSTR which is a wchar_t *.

But, as the userName contains for any unknown reason the user name twice, you need some different approach.

I recommend against statements like

    pszValidUser[ (userName.Length / 2) ] = 0;

without a comment. That is that kind of coding which any who must find out what goes wrong some years later lets fall into deep despair ...

Next issue is that I don't know a reason for retrieving memory by means of LocalLock. LocalLock is only provided for compatibility reasons with 16-bit Windows which might be necessary in the early 90's of the last century. Today it makes no sense, especially if you didn't do a LocalAlloc  before to get a valid handle and especially if you try to get more memory by calling LocalAllco again with the same handle ...

No, you should actively say good-bye to that kind of programming and do instead:  


#include <string>
using namespace std;
 
     wstring validUser = userName.Buffer;   // validUser now contains "VIAADMINVIAADMIN"
     int len = (int)validUser.length() ;
     if (len <= 2 || 
         (len%2) != 0 || 
         validUser.substr(0, len/2) != validUser.substr(len/2))
     {
         // not a valid userName 
         return false;
     }
     validUser = validUser.substr(0, len/2);
     if (validUser == pCred->pszUserName)
     {
          wstring validDomainUser = domainName.Buffer;
          validDomainUser += L"\\";
          validDomainUser += validUser;   // validDomainUser now contains mydomain\shashi 
                                          // if userName was "shashishashi" and domainName was "mydomain"
          // if you need a PCWSTR (const wchar_t*) you could do 
          const wchar_t* pcwz = validDomainUser.c_str();
          
          // if you need a PWSTR (cwchar_t*) you could do     
          PWSTR pcwz = new wchar_t[validDomainUser.size()+1];
          wcscpy(pcwz, validDomainUser.c_str());
          // use pcwz
          ....
          delete [] pcwz;
             
     }
  
            

Open in new window

0
 
evilrixSenior Software Engineer (Avast)Commented:
>> PWSTR pszValidUser = (PWSTR)(LocalLock(LocalAlloc(LHND,userName.Length + 2)
Since LocalAlloc allocates bytes and userName.length is (I'm guessing) the number of wide chars in the username shouldn't this actually be...

PWSTR pszValidUser = (PWSTR)(LocalLock(LocalAlloc(LHND,userName.Length * sizeof(wchar_t) + sizeof(wchar_t))

...?

Same issue for memcpy(pszValidUser,userName.Buffer, userName.Length); since memcpy copies bytes

Also, why don't you copy use wcsncpy?
http://msdn.microsoft.com/en-us/library/xdsywd25(VS.80).aspx

wcsncpy(pszValidUser,userName.Buffer, userName.Length);

0
 
yarabatiAuthor Commented:
Thanks a lot for the reply:

 if (wcsicmp(pszValidUser, pCred->pszUserName)== 0)
        {
             SysMessage(Event,"SUCCESS","Security alert: Match user %S\\%S with %S  at Client %s .",
                                pCred->pszDomain,
                                pCred->pszUserName,
                                pszValidUser,
                                szClientAddress);            
        }
        else
        {
           SysMessage(FatalError,"EXCEPTION","Security alert: Client %s mismatch for user %S\\%S with %S.",
                                szClientAddress,
                                pCred->pszDomain,
                                pCred->pszUserName,
                                pszValidUser);
                return FALSE;
        }

for below condition:
Security alert: Client <clientmachine>mismatch for user \domain1\VIAADMIN(THIS IS OLD VARIABLE) with VIAADMIN (NEW VARIABLE WHICH WE CREATED). More information is available at <link removed>


And before comparision, in the logs:

Comparing new user:VIAADMIN, with old user: domain1\VIAADMIN More information is available at <LINK>


I clearly see that new variable is not with the domain appended...

0
Upgrade your Question Security!

Your question, your audience. Choose who sees your identity—and your question—with question security.

 
yarabatiAuthor Commented:
Sorry, I see variables are matching.. both contains same value say, VIAADMIN in above...


Can you please help me with if condition block?
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> Can you please help me with if condition block?
I'm sorry but I don't follow what you're asking for help with.
0
 
yarabatiAuthor Commented:
Code:  if (wcsicmp(pszValidUser, pCred->pszUserName)== 0)

Both variables contain same value but this condition is failing and executing code under else block.
I don't know I am I doing correct or not, can you see is this the correct way to compare each other.

Any other option to check both are equal??
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> is this the correct way to compare each other.
Yes, that is correct.

How do you know they are identical? Have you tried looking at them both in the debugger? Often, when I've looked into these types of problem it's usually there is a very subtle difference (does one of them contain an embedded null maybe?). Also, try looking at the memory both of the pointers point to (again in the debugger).

Also, did you make the changes I suggested? The original code you posted is not really correct and as such might be the cause of your problem. Can you make those changes, test and if still an issue try looking using the debugger. If you still can't get it to work please post all the code, verbatim, back here for review.
0
 
yarabatiAuthor Commented:
Thanks a lot for swift responses.

I have added your code, please see the code:

And Output I received is :

Comparing new user:VIAADMIN, with old user:domain1\VIAADMIN More information is available at <SOME LINK>

Security alert: Client LOANER2-LHR2 mismatch for user \domain1\VIAADMIN with VIAADMIN. More information is available at <SOME LINK>


You are right, both variables are not identical. But we copied it in right way, confused :(



		PWSTR pszValidUser = (PWSTR)(LocalLock(LocalAlloc(LHND,userName.Length * sizeof(wchar_t) + sizeof(wchar_t))));
		wcsncpy(pszValidUser,userName.Buffer, userName.Length);
        pszValidUser[ (userName.Length / 2) ] = 0;
 
		SysMessage(Event,"SUCCESS","Comparing new user:%S, with old user:%S",
                                pszValidUser,
								pCred->pszUserName);
 
        if (wcsicmp(pszValidUser, pCred->pszUserName)== 0)
        {
             SysMessage(Event,"SUCCESS","Security alert: Match user %S\\%S with %S  at Client %s .",
                                pCred->pszDomain,
                                pCred->pszUserName,
                                pszValidUser,
                                szClientAddress);             
        }
        else
        {
           SysMessage(FatalError,"EXCEPTION","Security alert: Client %s mismatch for user %S\\%S with %S.",
                                szClientAddress,
                                pCred->pszDomain,
                                pCred->pszUserName,
                                pszValidUser);
		    return FALSE;
        }

Open in new window

0
 
itsmeandnobodyelseCommented:
>>>> you are right, both variables are not identical. But we copied it in right way, confused :(

    pszValidUser[ (userName.Length / 2) ] = 0;

That stament is wrong. It should be

     pszValidUser[userName.Length] = 0;

Or better do

  wcscpy(pszValidUser,userName.Buffer);

what would copy the binary zero at end of string.



0
 
itsmeandnobodyelseCommented:
BTW, why do you need a copy of the wide string? The userName obviously is a string class type. So you can do


     if (userName == pszUserName)
     {
         ...


0
 
yarabatiAuthor Commented:

If I change code to:      pszValidUser[userName.Length] = 0;
I am getting value as - VIAADMINVIAADMIN :(


For below Code:
            PWSTR pszValidUser = (PWSTR)(LocalLock(LocalAlloc(LHND,userName.Length * sizeof(wchar_t) + sizeof(wchar_t))));
            wcsncpy(pszValidUser,userName.Buffer, userName.Length);
        pszValidUser[ (userName.Length / 2) ] = 0;

            PWSTR tmpDomain = (PWSTR)(LocalLock(LocalAlloc(LHND,domainName.Length + 2)));
            memcpy(tmpDomain,domainName.Buffer, domainName.Length);
            tmpDomain[ (domainName.Length / 2) ] = 0


I will get two variables pszValidUser & tmpDomain with their user names and domain names.
I would like to append tmpDomain value infront of pszValidUser.

Say if pszValidUser=shashi and tmpDomain=mydomain

I need newUser=mydomain\shashi, any help from you?
0
 
yarabatiAuthor Commented:
Re:
BTW, why do you need a copy of the wide string? The userName obviously is a string class type. So you can do


     if (userName == pszUserName)
     {
         ...

pszUserName - It contains some user name before one function runs. We know even this returns same user name.
After that function runs we will get new user name. Then we will override pszUserName with new user name.
We want to check whether the function returned user name is same as pszUserName. If not throw error.

So, here what I am doing is Iam taking the new user name to another variable pszValidUser and checking with it's old value before do overriding...

0
 
yarabatiAuthor Commented:

Any help from any one, for below question?

For below Code:

            PWSTR pszValidUser = (PWSTR)(LocalLock(LocalAlloc(LHND,userName.Length * sizeof(wchar_t) + sizeof(wchar_t))));
            wcsncpy(pszValidUser,userName.Buffer, userName.Length);
        pszValidUser[ (userName.Length / 2) ] = 0;

            PWSTR tmpDomain = (PWSTR)(LocalLock(LocalAlloc(LHND,domainName.Length + 2)));
            memcpy(tmpDomain,domainName.Buffer, domainName.Length);
            tmpDomain[ (domainName.Length / 2) ] = 0


I will get two variables pszValidUser & tmpDomain with their user names and domain names.
I would like to append tmpDomain value infront of pszValidUser.

Say if pszValidUser=shashi and tmpDomain=mydomain

I need newUser=mydomain\shashi, any help from you?
0
 
itsmeandnobodyelseCommented:
BTW, what string class is the username and domainName?

If it has a substring function you may do all things I did using the original string class. That isn't a vote against wstring class but only a vote to keep things simple as long as there are no invincible restrictions.
0
 
yarabatiAuthor Commented:
I am sorry for slight delay in getting back to you:

I have below variable declared and used:
PAE_UNICODE_STRING userName;

I want to have that to be converted to PWSTR, so that comparison& validation work makes easy to me?
Any ideas?
0
 
yarabatiAuthor Commented:

Here is the declaration:

typedef struct _PAE_UNICODE_STRING {
            USHORT Length;
            USHORT MaximumLength;
            [size_is(MaximumLength / 2), length_is((Length) / 2) ] USHORT * Buffer;
      } PAE_UNICODE_STRING;

0
 
itsmeandnobodyelseCommented:
The Buffer member of the structure PAE_UNICODE_STRING is a PWSTR. No conversion necessary. You best use a wstring as a temporary as I showed in the aabove code.
0
 
yarabatiAuthor Commented:
Thanks itsmeannobodyelse.

I tried with below source code:

wstring prevUser = userName.Buffer;
            int le = (int)prevUser.length() ;
            prevUser = prevUser.substr(0, le/2);
            PWSTR prevUserPWSTR = new wchar_t[prevUser.size()+1];

Tried printing the value prevUserPWSTR, but see in the output as nothing means no string inside :(

Any help?
0
 
yarabatiAuthor Commented:

I am really sorry guys, if I sound so silly. I am completely newbie to this C++ stuff. I have bit understanding from academic background, but as you know that's not worthy in real time...

I would really appreciate your help from here, and many thanks for that!
0
 
itsmeandnobodyelseConnect With a Mentor Commented:
>>>> PWSTR prevUserPWSTR = new wchar_t[prevUser.size()+1];
That is only an allocation of a new buffer. You don't need that.

Print  prevUser or prevUser.c_str() e. g. by

  wcout << L"Prev User = " << prevUser;

The prevUser.c_str()  is a PWCSTR what is a pointer to const wide char array.

If you need a non-const pointer PWSTR (???) you do

PWSTR prevUserPWSTR = new wchar_t[prevUser.size()+1];
wcscpy(prevUserPWSTR, prevUser.c_str() );

But I am really sure that it is not necessary.  
0
 
yarabatiAuthor Commented:
Thanks a lot itsmeannobody else....

Finally it got resolved now... thanks a lot for your help!
0
 
evilrixSenior Software Engineer (Avast)Commented:
Points well earned I think Alex :)
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.