NetGroupGetUsers strange errors on Win64 on buffered reads

[Davide Angeli 44]

Hi all, this is not directly Delphi related, just want to share if anyone will experience something like this.

 

I'm using NetGroupGetUser api (from netapi32.dll) to retrive names and data of the domain users. It worked well for a while, but now (maybe some recent Windows Updates), in win64 applications, it returns me strange results or access violations. Same code compiled in Win32 instead always worked fine. Attached is a sample that I've found online (stackoverflow) that I'm using to do some tests. The original program was designed to retrieve data in buffers (1024byte). Doing that, in my case, if users number exceeds the buffer, the second call of NetGroupGetUser raise an access violation in samlib.dll (running on a Windows 2019 server). I've solved calling the api once using MAX_PREFERRED_LENGTH (=DWORD(-1)) instead of the original 1024 byte buffer size. I could not find documentation about differences in declaring the api for win32 or win64... It seems some kind of mess with the api parameters in 64 bit or a bug of 64bit version of netapi32.dll.

TestDomainUserConsole.zip

[Lars Fosdal 1792]

Windows 10, 64-bit

Compile to 32-bit target - runs as expected

Compile to 64-bit target - unable to create process

[David Heffernan 2345]

Would be nice to look at code directly in the post rather than have to download some ZIP file

[Lars Fosdal 1792]

@Davide Angeli - BTW- Why don't you use the AD APIs instead?

[Davide Angeli 44]

3 hours ago, David Heffernan said:

Would be nice to look at code directly in the post rather than have to download some ZIP file

{$WARN SYMBOL_PLATFORM OFF}

program DomainGroupGetUsersTest;

{$APPTYPE CONSOLE}

uses
  SysUtils, Windows, Classes;

const
    netapi32lib = 'netapi32.dll';

type
    PGroupUsersInfo0 = ^TGroupUsersInfo0;
    _GROUP_USERS_INFO_0 = record
      grui0_name: LPWSTR;
    end;
    TGroupUsersInfo0 = _GROUP_USERS_INFO_0;
    GROUP_USERS_INFO_0 = _GROUP_USERS_INFO_0;

    NET_API_STATUS = DWORD;
    LPBYTE = ^BYTE;

function NetApiBufferFree (Buffer: Pointer): NET_API_STATUS; stdcall;
                                                           external netapi32lib;
function NetGroupGetUsers (servername: LPCWSTR; groupname: LPCWSTR;
    level: DWORD; var bufptr: LPBYTE; prefmaxlen: DWORD; var entriesread: DWORD;
    var totalentries: DWORD; ResumeHandle: PDWORD): NET_API_STATUS; stdcall;
                                                           external netapi32lib;

function DomainGroupGetUsers (const sGroup: WideString;
                              const UserList: TStrings;
                              const sLogonServer: WideString) : Boolean;
{ "sLogonServer" must be prefixed with "\\".
  "sGroup" must contain the group name only. }

type
    TaUserGroup = array of TGroupUsersInfo0;

const
    PREF_LEN = 1024;
//    MAX_PREFERRED_LENGTH = DWORD(-1);

var
    pBuffer : LPBYTE;
    i : Integer;
    Res : NET_API_STATUS;
    dwRead, dwTotal, hRes : DWord;

begin
    Assert (sGroup <> '');
    Assert (sLogonServer <> '');
    Assert (UserList <> NIL);

    UserList.Clear;
    Result := true;
    hRes := 0;

    repeat
        writeln('hres = '+IntToStr(hRes));

//        Res := NetGroupGetUsers (PWideChar (sLogonServer), PWideChar (sGroup),
//                                 0, pBuffer, MAX_PREFERRED_LENGTH, dwRead, dwTotal,
//                                 PDWord (@hRes));
        Res := NetGroupGetUsers (PWideChar (sLogonServer), PWideChar (sGroup),
                                 0, pBuffer, PREF_LEN, dwRead, dwTotal,
                                 PDWord (@hRes));


        writeln('dwRead = '+IntToStr(dwRead));
        writeln('dwTotal = '+IntToStr(dwTotal));

        if (Res = Error_Success) or (Res = ERROR_MORE_DATA) then
        begin
            if (dwRead > 0) then
                for i := 0 to dwRead - 1 do
                    with TaUserGroup (pBuffer) [i] do
                        UserList.Add (grui0_name);

            NetApiBufferFree (pBuffer);
        end { if }
        else Result := false;
    until (Res <> ERROR_MORE_DATA);
end; { DomainGroupGetUsers }


var
    UserList : TStringList;
    iIndex : Integer;

begin
    UserList := TStringList.Create;

    try
        DomainGroupGetUsers ('Domain Users', UserList,
                             GetEnvironmentVariable ('LOGONSERVER'));

        for iIndex := 0 to UserList.Count - 1 do
            WriteLn (UserList [iIndex]);

    finally
        UserList.Free;
    end; { try / finally }

    if (DebugHook <> 0) then
    begin
        WriteLn;
        Write ('Press [Enter] to continue ...');
        ReadLn;
    end; { if }
end.

 

[Davide Angeli 44]

2 hours ago, Lars Fosdal said:

@Davide Angeli - BTW- Why don't you use the AD APIs instead?

A few years ago I needed to get that info and when I googled I only found examples like that posted.

 

I don't know AD Api. Is there a Delphi wrapper?

Edited October 20, 2022 by Davide Angeli

[Davide Angeli 44]

3 hours ago, Lars Fosdal said:

Windows 10, 64-bit

Compile to 32-bit target - runs as expected

Compile to 64-bit target - unable to create process

I'm compiling it both 32bit and 64bit and they work both here.

I'm on Windows 10 64bit with Delphi 11.2.

 

The 64bit compiled exe I've tested also on Win11 (22H1) and on WinServer 2019 and Winserver 2022 and it works everywhere.

 

 

[Lars Fosdal 1792]

Check out unit web.win.AdsTypes:

Here is some code I wrote to find out if a user had an AD membership.

unit ActiveDSUtil;

/// Written by Lars Fosdal, 16 DEC 2014
/// Note that calling AD functions is slow.

interface
uses
  Classes, SysUtils, ActiveX, ActiveDS_tlb, web.win.adstypes;

type
  TADGroupList = array of String;
  TAnonParamFunc<TA,TR> = reference to function (const v:TA):TR;

  /// <summary> Enumerates the group memberships of an AD user </summary>
  function EnumADUserGroupMemberships(const aDomain, aUser: String; EnumHandler: TAnonParamFunc<IAdsGroup, Boolean>):Boolean;

  /// <summary> Returns a list of all AD groups for an AD user </summary>
  function GetADUserGroupMemberships(const aDomain, aUser: String):TStringList;

  /// <summary> Checks if an AD user is member of one or more specific groups</summary>
  function UserHasADGroupMembership(const aDomain, aUser: String; const GroupList: TAdGroupList): Boolean;


implementation

function EnumADUserGroupMemberships(const aDomain, aUser: String; EnumHandler: TAnonParamFunc<IADsGroup, Boolean>):Boolean;
var
  hr: HREsult;
  User: IADsUser;
  Enum: IEnumVariant;
  varGroup: OleVariant;
  EnumHelper: LongWord;
begin
  Result := False;
  CoInitialize(nil);
  try
    hr := ADsGetObject('WinNT://'+aDomain+'/'+aUser+',user',IID_IADsUser3 , User);
    if not Failed(hr)
    then begin
      try
        Enum := User.Groups._NewEnum as IEnumVariant;
        while Assigned(Enum) and (Enum.Next(1, varGroup, EnumHelper) = S_OK)
        do begin
          try
            if EnumHandler(IDispatch(varGroup) as IADsGroup)
             then EXIT(True);
          finally
            VariantClear(varGroup);
          end;
        end;
      finally
        User := nil;
      end;
    end;
  finally
    CoUninitialize;
  end;
end;

function GetADUserGroupMemberships(const aDomain, aUser: String):TStringList;
var
  List: TStringList;
begin
  List := TStringList.Create;
  List.BeginUpdate;
  try
    EnumADUserGroupMemberships(aDomain, aUser,
      function(const Group: IAdsGroup):Boolean
      begin
        Result := False;
        List.Add(Group.Name + ' ' + Group.Class_);
      end);
  finally
    List.Sort;
    List.Insert(0, aDomain +'\'+ aUser);
    List.EndUpdate;
    Result := List;
  end;
end;

function UserHasADGroupMembership(const aDomain, aUser: String; const GroupList: TAdGroupList): Boolean;
begin
  Result := EnumADUserGroupMemberships(aDomain, aUser,
      function(const Group: IAdsGroup):Boolean
      var
        GroupName: String;
      begin
        Result := False;
        for GroupName in GroupList
        do begin
          Result := CompareText(GroupName, Group.Name) = 0;
          if Result
           then Break; // Return true for first match
        end;
      end);
end;

end.

 

[Lars Fosdal 1792]

8 minutes ago, Davide Angeli said:

I'm compiling it both 32bit and 64bit and they work both here.

I'm on Windows 10 64bit with Delphi 11.2.

 

The 64bit compiled exe I've tested also on Win11 (22H1) and on WinServer 2019 and Winserver 2022 and it works everywhere.

The 64-bit simply refuses to run, unless I run it elevated.

Then I get

/--- Started ---
hres = 0
dwRead = 35
dwTotal = 712
hres = 3608717696
EAccessViolation: Access violation at address 00007FFBCE7CB0EC in module 'SAMCLI.DLL'. Read of address 00000000D718A9A8
Press Enter:

Perhaps there are further access rights that come into play?

 

[Davide Angeli 44]

12 minutes ago, Lars Fosdal said:

The 64-bit simply refuses to run, unless I run it elevated.

Then I get

Yes I'm working as admin so I can run without elevation. I suppose you have several users in the domain so it need to call the api more than one time and on second call you get AV. It is the problem for which I started this topic. If you use the MAX_PREFERED_LENGHT version (comment/uncomment instructions)  does it work? For me yes and some weeks ago worked also the original version..

Edited October 20, 2022 by Davide Angeli

[Lars Fosdal 1792]

Confirmed.  It worked running elevated with the MAX_PREFERRED_LENGTH and returned some 16k+ users.

[Lars Fosdal 1792]

6 minutes ago, Davide Angeli said:

I'm working as admin so I could run without elevation.

Working as admin by default would be considered a massive no-no by my company.

[David Heffernan 2345]

ResumeHandle should be PDWORD_PTR. Probably other types are declared incorrectly. Check all declarations against the header files. 

 

Ask yourself which is more likely. Is it a bug in your code, or a bug in the Windows code? 

Edited October 20, 2022 by David Heffernan

[Davide Angeli 44]

3 hours ago, David Heffernan said:

ResumeHandle should be PDWORD_PTR. Probably other types are declared incorrectly. Check all declarations against the header files. 

 

Ask yourself which is more likely. Is it a bug in your code, or a bug in the Windows code? 

1. The code I posted is not my code. I took that example on stackoverflow here: https://stackoverflow.com/questions/34870232/list-all-users-of-an-ad-group-in-delphi. I post it just to provide a quick example that leads to the same error that I'm eperiencing (that's the reason why I posted the zip file with the project ready to test...). My code is more complex (I need more information about that api so I used anoother data structure). In my project I'm using JclWin32 to call that api and there the function is declared like this:
   function NetGroupGetUsers(servername, groupname: LPCWSTR; level: DWORD; var bufptr: PByte; prefmaxlen: DWORD; entriesread, totalentries: LPDWORD; ResumeHandle: PDWORD_PTR: NET_API_STATUS;
   The result is the same.
    
2. Are you so sure that Windows is bug free? My code run smootlhy for at least a couple of years on a Windows 2019 server with hundreds of domain users. On that server were locked the Windows updates for a while because one of them created problems accessing via RDP. Last week we unlock the updates and boom! That piece of code now raises the access violation for which we are discussing. Sure it could be a coincidence.
    
3. According to Microsoft documentation the declaration of that api is this one (see more at https://learn.microsoft.com/en-us/windows/win32/api/lmaccess/nf-lmaccess-netgroupgetusers)
   NET_API_STATUS NET_API_FUNCTION NetGroupGetUsers(
     [in]      LPCWSTR    servername,
     [in]      LPCWSTR    groupname,
     [in]      DWORD      level,
     [out]     LPBYTE     *bufptr,
     [in]      DWORD      prefmaxlen,
     [out]     LPDWORD    entriesread,
     [out]     LPDWORD    totalentries,
     [in, out] PDWORD_PTR ResumeHandle
   );
   and JclWin32 declaration seems correct but compiling it Win64 raises that AV.
   So it's a bug of JclWin32 code or a bug in Windows? I dont know

 

Edited October 20, 2022 by Davide Angeli

[David Heffernan 2345]

It's not going to be a defect in Windows. It's going to be a defect in the code you are compiling. That is your starting mindset. 

 

Perhaps somebody else wants to debug your large and complex code. But if you made a minimal reproduction and posted it here I'd expect a better chance of engagement. 

Edited October 21, 2022 by David Heffernan

[Lars Fosdal 1792]

It is a very old legacy Windows function, though...

[Davide Angeli 44]

2 hours ago, David Heffernan said:

Perhaps somebody else wants to debug your large and complex code. But if you made a minimal reproduction and posted it here I'd expect a better chance of engagement. 

I know that my english is terrible but I suppose you have misunderstood the reason of this topic. I'm not looking for someone to debug my super beatiful large and complex code written in the better way possible and bla bla... As I wrote, I've already solved the problem with the workaround that I have posted (it's not the best solution but it works) and now with @Lars Fosdal kind answers I also could try other modern ways to reach my goal (like AD apis).

 

I wrote this topic if could help somebody having the same issue. If this kind of topics are not accepted here I'll be careful not to do it again. Sorry

[Davide Angeli 44]

1 hour ago, Lars Fosdal said:

It is a very old legacy Windows function, though...

You are right but as you know things that work we don't touch as long as they work! Now I'll dedicate time to modernize that piece of code.

[David Heffernan 2345]

1 hour ago, Davide Angeli said:

I know that my english is terrible but I suppose you have misunderstood the reason of this topic. I'm not looking for someone to debug my super beatiful large and complex code written in the better way possible and bla bla... As I wrote, I've already solved the problem with the workaround that I have posted (it's not the best solution but it works) and now with @Lars Fosdal kind answers I also could try other modern ways to reach my goal (like AD apis).

 

I wrote this topic if could help somebody having the same issue. If this kind of topics are not accepted here I'll be careful not to do it again. Sorry

I though we were talking about the cause of the access violations in your code. But if I've misunderstood, I'm sorry. 

[Davide Angeli 44]

2 minutes ago, David Heffernan said:

I though we were talking about the cause of the access violations in your code. But if I've misunderstood, I'm sorry. 

No problem!

 

Discover the cause of the acces violation could be maybe a good exercise for some Windows api guru. I've dedicated it some hours yesterday trying to change the way to declare the parameters of the api but I could not reach a solution. I've tried several combinations even going against what is stated in the api documentation. There is something strange tha I'm missing on that api because the totalentries out parameter is always a non sense value called in win64. In win32 works as apected. So maybe there is something not documented in the Windows api or maybe, as that api si considered very old, maybe the win64 part could be buggy (just a my guess) or maybe could be something wrong using it from a Delphi win64 program (I've tested it both 10.4.2 and 11.2 but the problem is the same).

[Lars Fosdal 1792]

Perhaps check that the types used in the declaration of the API doesn't inappropriately change size on the Delphi side depending on if we compile for 32-bit vs 64-bit?