2

After creating a user account with NetUserAdd, I found out that I would need to use NetLocalGroupAddMembers to add the user to the Users group, so I called CreateWellKnownSid to get the Users' SID, LookupAccountSid to get the string name from that SID and pass it to NetLocalGroupAddMembers.

I also needed to specify the user name, but the function required domain\name format as level 3 (LOCALGROUP_MEMBERS_INFO_3), but I did not have that. I decided to call LookupAccountName to get the user name SID and pass it to level 0 (LOCALGROUP_MEMBERS_INFO_0).

This is how I did it:

//LocalAlloc
    UINT memAttributes = LMEM_FIXED;
    SIZE_T sidSize = SECURITY_MAX_SID_SIZE;

//LookupAccountName
    PSID accountSID;
    SID_NAME_USE typeOfAccount;

//NetLocalGroupAddMembers
    NET_API_STATUS localGroupAdd;
    DWORD levelOfData = 0;  //LOCALGROUP_MEMBERS_INFO_0
    LOCALGROUP_MEMBERS_INFO_0 localMembers;
    DWORD totalEntries = 0;



//Allocate memory for LookupAccountName
        if (!(accountSID = LocalAlloc(memAttributes, sidSize)))
        {
            wprintf(L"\nMemory allocation for account SID failed: \n");
            ShowError(GetLastError());
            exit(1);

        }


if (!LookupAccountNameW(NULL, argv[1], accountSID,
                                    (LPDWORD)&sidSize, NULL, 0, &typeOfAccount))
            {
                fwprintf(stderr, L"Error getting SID from name: \n");
                ShowError(GetLastError());
                return 1;

            }

//Here I should be able to use NetLocalGroupAddMembers
            //to add the user passed as argument to the Users group. 
            localMembers.lgrmi0_sid = accountSID;

            localGroupAdd = NetLocalGroupAddMembers(NULL, name, levelOfData, (LPBYTE)&localMembers, totalEntries);

            if (localGroupAdd != NERR_Success)
            {
                fwprintf(stderr, L"Error adding member to the local group: \n");
                ShowError(GetLastError());
                return 1;
            }
            else
            {
                wprintf(L"\nUser %s has been successfully added.\n", argv[1]);

            }

This is the error I am getting:

Exception thrown at 0x743F059A (sechost.dll) in UserCreator.exe: 0xC0000005: Access violation writing location 0x00000000.

Any clues?

mkrieger1
  • 19,194
  • 5
  • 54
  • 65
Sergio Calderon
  • 837
  • 1
  • 13
  • 32
  • 3
    Always try to remove casts like (LPDWORD) because they tend to hide errors. It is not the problem in this case but could be a issue in other cases in 64-bit programs. – Anders Jan 03 '18 at 21:18
  • 1
    In particular, the type of `sidSize` should be `DWORD`, not `SIZE_T` (at least, if you want to use it also for `LookupAccountNameW`). – Matteo Italia Jan 03 '18 at 21:21

1 Answers1

1

The ReferencedDomainName parameter is not actually optional.

LPCTSTR machine = NULL, username = /*TEXT("Anders")*/ argv[1];
TCHAR domain[MAX_PATH];
BYTE accountSIDbuf[SECURITY_MAX_SID_SIZE];
PSID accountSID = (PSID) accountSIDbuf;
DWORD cbSid = SECURITY_MAX_SID_SIZE, cchRD = MAX_PATH;
SID_NAME_USE snu;

if (!LookupAccountName(machine, username, accountSID, &cbSid, domain, &cchRD, &snu))
{
    printf("Error %u\n", GetLastError());
    return ;
}

LPTSTR sidstr;
if (!ConvertSidToStringSid(accountSID, &sidstr)) { return ; }
_tprintf(_T("SID of %s\\%s is %s\n"), domain, username, sidstr);
LocalFree(sidstr);

Another problem with your code is ShowError(GetLastError()); You cannot use GetLastError() after calling some other function. Rewrite as

DWORD error = GetLastError();
fwprintf(stderr, L"Error getting SID from name: \n");
ShowError(error);

but in this case even that is wrong because NetLocalGroupAddMembers does not call SetLastError, it just returns the error code directly.

Edit:

Just to clarify the paramter usage; If you want to query the required size of the domain buffer you can do this:

DWORD cchRD = 0;
LookupAccountName(..., NULL, &cchRD, &snu); // The function is still going to report failure
LPTSTR domain = malloc(cchRD * sizeof(*domain));
LookupAccountName(..., domain, &cchRD, &snu);

In my example I avoid this by just passing in a buffer that is "large enough".

Anders
  • 97,548
  • 12
  • 110
  • 164
  • I changed the first parameter to add the machine variable, but still getting the same error. Like this: `LPCWSTR machine = NULL; if (!LookupAccountNameW(machine, argv[1], accountSID, (LPDWORD)&sidSize, NULL, 0, &typeOfAccount)) { fwprintf(stderr, L"Error getting SID from name.\n"); return 1; } ` – Sergio Calderon Jan 03 '18 at 21:32
  • 2
    Actually, the `ReferencedDomainName` parameter *is* optional. It's the `cchReferencedDomainName` that isn't, and that's what's causing the AV, as the API is trying to write the required buffer size to that address, as [documented](https://msdn.microsoft.com/en-us/library/windows/desktop/aa379159.aspx). – IInspectable Jan 03 '18 at 21:34
  • @IInspectable, yes, that is how it is documented but I'm not sure if it has always been true on every system but my memory might be wrong. Even if it is optional all the way back to Win 95, the function is going to "fail" if the string parameter is NULL and then you have to allocate and call it again. Might as well just supply a large buffer in the first place. – Anders Jan 03 '18 at 21:38
  • 2
    The documentation need not be true for every version of Windows ever released. It references only supported versions of Windows (that is, Windows Vista and later, as of today). The issue here is, that the OP failed to provide a valid pointer to a `DWORD` variable, receiving the required size. Passing `nullptr` as the `ReferenceDomainName` is valid, and follows a common pattern in the API. – IInspectable Jan 03 '18 at 21:42
  • @IInspectable, I have changed the `cchReferencedDomainName` and worked, but now it is telling me that *The data area passed to a system call is too small*. Does the `sidSize` need to be bigger? Let me know if I need to put this as different question. Thank you both! – Sergio Calderon Jan 03 '18 at 21:47
  • 1
    @SergioCalderon When cchReferencedDomainName is 0 (that is, cchReferencedDomainName=0 and calling the function with &cchReferencedDomainName) you are asking the function to report the required size. The function will return failure and you need to allocate a buffer and call the function again. In my example I just pass a buffer that is large enough in the first place. For the function to succeed you must pass a valid buffer for the ReferencedDomainName parameter. – Anders Jan 03 '18 at 22:28
  • @Anders, did you try using those buffer sizes? I just tried, but still getting same error. I might need to use the function twice so I can get the needed size. Gonna try and get back here. Thanks for your answers! – Sergio Calderon Jan 04 '18 at 02:02
  • There is no way your domain/computer name is longer than 259 characters. And yes, I tested my code. – Anders Jan 04 '18 at 02:15