View Issue Details

IDProjectCategoryView StatusLast Update
0036368FPCPackagespublic2019-12-02 16:14
ReporterLukasAssigned ToMarco van de Voort 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformOSWindows 10OS Version
Product Version3.3.1Product Build 
Target VersionFixed in Version 
Summary0036368: JwaAccCtrl uses enum size 1 instead of 4
DescriptionJwaAccCtrl contains various records and enums for interfacing with the WinApi. The C++ WinApi assumes that all enums are 4 bytes in size which also influences its assumptions about record size/alignment. However, JwaAccCtrl's Enums are of size 1 which breaks interfacing with certain WinApi functions, like SetEntriesInAclA.
Steps To ReproduceI used the latest Windows i386 fpc (ftp://ftp.freepascal.org/pub/fpc/snapshot/trunk/i386-win32/fpc-3.3.1.i386-win32.zip), but I was also able to reproduce this bug on x86_64 fpc and the latest stable release.

To demonstrate the issue I use the attached test.pas and compile it using: C:\Users\Administrator\Downloads\fpc-3.3.1.i386-win32\bin\i386-win32\fpc.exe -Fu"C:\Users\Administrator\Downloads\fpc-3.3.1.i386-win32\units\i386-win32\*" test.pas

When running it I get the following output:

>test.exe
EXPLICIT_ACCESS_A size: 20 ENUM SIZE: 1

I would expect EXPLICIT_ACCESS_A to be of size 32 on i386 because all Enums inside of it should be of size 4. This breaks interfacing with WinApi functions like in this case SetEntriesInAclA (https://docs.microsoft.com/en-us/windows/win32/api/aclapi/nf-aclapi-setentriesinacla) which assumes a size of 32. Using this function with a JwaAccCtrl record will always result in Error 87 (Invalid Parameter). I also attached an example function which is affected by this issue in acltest.pas:

> C:\Users\Administrator\Downloads\fpc-3.3.1.i386-win32\bin\i386-win32\fpc.exe -Fu"C:\Users\Administrator\Downloads\fpc-3.3.1.i386-win32\units\i386-win32\*" acltest.pas
> acltest.exe
Error in SetEntriesInAcl: 87
Fail
EXPLICIT_ACCESS_A size: 20

TagsNo tags attached.
Fixed in Revision43608
FPCOldBugId
FPCTarget-
Attached Files
  • test.pas (217 bytes)
    program test;
    
    {$PACKENUM 4}
    
    uses
      jwaaccctrl,
      SysUtils;
    
    begin
      WriteLn(Format('EXPLICIT_ACCESS_A size: %d ENUM SIZE: %d', [SizeOf(jwaaccctrl.EXPLICIT_ACCESS_A), SizeOf(jwaaccctrl.ACCESS_MODE)]));
    end.
    test.pas (217 bytes)
  • acltest.pas (6,537 bytes)
    program acltest;
    
    {$MODE DELPHI}
    
    uses
      sysutils,
      JwaWinNT,
      JwaAclApi,
      JwaAccCtrl,
      JwaSDDL,
      jwawinbase,
      jwawinsta,
      jwawintype,
      jwawinerror;
    
    function SetFilePermissionForRunAs(filename: string): boolean;
    var
      sid: JwaWinNT.PSID;
      groupSid: JwaWinNT.PSID;
      sidSize: DWORD;
      nameUse: JwaWinNT.SID_NAME_USE;
      refDomain: LpStr;
      refDomainSize: DWORD;
      dolocalfree: boolean;
      procToken: HANDLE;
      changedSecurityInfo: DWORD;
      acl: jwawinnt.PACL;
    
      function SetPrivilege(token: HANDLE; privilege: PChar; state: boolean): boolean;
      var
        luid: jwawintype._LUID;
        tp: jwawinnt.TOKEN_PRIVILEGES;
      begin
        if not jwawinbase.LookupPrivilegeValue(nil, privilege, luid) then
          Result := false
        else
        begin
          FillChar(tp, sizeOf(tp), #0);
          tp.privilegeCount := 1;
          tp.Privileges[0].Luid := luid;
          if state then
            tp.Privileges[0].Attributes := SE_PRIVILEGE_ENABLED
          else
            tp.Privileges[0].Attributes := SE_PRIVILEGE_REMOVED;
    
          Result := AdjustTokenPrivileges(token, LongBool(false), @tp, sizeOf(tp), nil,nil);
        end
      end;
    
      function SetupAccess(owner: jwawinnt.PSID; var acl: jwawinnt.PACL): bool;
      const
        EA_COUNT = 3;
      var
        sidAuthWorld: jwawinnt.SID_IDENTIFIER_AUTHORITY;
        sidAuthNT: jwawinnt.SID_IDENTIFIER_AUTHORITY;
        everyoneSID: jwawinnt.PSID;
        adminSID: jwawinnt.PSID;
        ea: Array[0..(EA_COUNT-1)] of jwaaccctrl.EXPLICIT_ACCESS_A;
        status: jwawintype.DWORD;
      begin
        try
          begin
            sidAuthWorld := jwawinnt.SECURITY_WORLD_SID_AUTHORITY;
            sidAuthNT := jwawinnt.SECURITY_NT_AUTHORITY;
    
            if not (jwawinbase.AllocateAndInitializeSid(@sidAuthWorld, 1, jwawinnt.SECURITY_WORLD_RID,
                0, 0, 0, 0, 0, 0, 0, everyoneSID)
              and AllocateAndInitializeSid(@sidAuthNT, 2, jwawinnt.SECURITY_BUILTIN_DOMAIN_RID,
              jwawinnt.DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0, adminSID)) then
            begin
              WriteLn('Could not allocate SIDs: ' + SysErrorMessage(getLastError()));
              Result := false;
            end
            else
            begin
              jwawinbase.ZeroMemory(@ea, EA_COUNT * sizeOf(EXPLICIT_ACCESS_A));
    
              ea[0].grfAccessPermissions := GENERIC_ALL;
              ea[0].grfAccessMode := DENY_ACCESS;
              ea[0].grfInheritance := NO_INHERITANCE;
              ea[0].Trustee.MultipleTrusteeOperation := NO_MULTIPLE_TRUSTEE;
              ea[0].Trustee.pMultipleTrustee := nil;
              ea[0].Trustee.TrusteeForm := TRUSTEE_IS_SID;
              ea[0].Trustee.TrusteeType := TRUSTEE_IS_WELL_KNOWN_GROUP;
              ea[0].Trustee.ptstrName := pointer(everyoneSID);
    
              ea[1].grfAccessPermissions := GENERIC_ALL;
              ea[1].grfAccessMode := SET_ACCESS;
              ea[1].grfInheritance := NO_INHERITANCE;
              ea[1].Trustee.MultipleTrusteeOperation := NO_MULTIPLE_TRUSTEE;
              ea[1].Trustee.pMultipleTrustee := nil;
              ea[1].Trustee.TrusteeForm := TRUSTEE_IS_SID;
              ea[1].Trustee.TrusteeType := TRUSTEE_IS_GROUP;
              ea[1].Trustee.ptstrName := pointer(adminSID);
    
              ea[2].grfAccessPermissions := GENERIC_ALL;
              ea[2].grfAccessMode := SET_ACCESS;
              ea[2].grfInheritance := NO_INHERITANCE;
              ea[2].Trustee.MultipleTrusteeOperation := NO_MULTIPLE_TRUSTEE;
              ea[2].Trustee.pMultipleTrustee := nil;
              ea[2].Trustee.TrusteeForm := TRUSTEE_IS_SID;
              ea[2].Trustee.TrusteeType := TRUSTEE_IS_USER;
              ea[2].Trustee.ptstrName := pointer(owner);
    
              status := jwaaclapi.SetEntriesInAclA(2, @ea, nil, acl);
              if status = ERROR_SUCCESS then
                Result := true
              else
              begin
                WriteLn('Error in SetEntriesInAcl: ' + IntToStr(status));
                Result := false;
              end;
            end;
          end
        finally
          If Assigned(everyoneSID) then
            jwawinbase.FreeSID(everyoneSID);
          If Assigned(adminSID) then
            jwawinbase.FreeSID(adminSID);
        end
      end;
    begin
        try
          begin
            sidSize := 0;
            refDomainSize := 0;
            doLocalFree := false;
    
            procToken := INVALID_HANDLE_VALUE;
    
            groupSid := nil;
            sid := nil;
            changedSecurityInfo := jwawinnt.OWNER_SECURITY_INFORMATION
              or jwawinnt.DACL_SECURITY_INFORMATION;
    
              jwawinbase.LookupAccountNameA(nil, 'Administrator', nil,
                sidSize, nil, refDomainSize, nameUse);
    
              GetMem(sid, sidSize);
              GetMem(refDomain, refDomainSize);
    
              Result := jwawinbase.LookupAccountNameA(nil, 'Administrator',
                sid, sidSize, refDomain, refDomainSize, nameUse);
    
              if not SetupAccess(sid, acl) then
              begin
                Result := false;
              end
              else if jwaaclapi.SetNamedSecurityInfoA(PChar(filename),
                JwaAccCtrl.SE_FILE_OBJECT, changedSecurityInfo, sid, groupSid, acl, nil)
                <> ERROR_SUCCESS then
              begin
                // if it fails we are probably missing the SE_RESTORE_NAME privilege, so we retry
                if (not OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, procToken)) or
                   (not SetPrivilege(procToken, SE_RESTORE_NAME, true)) then
                  Result := false
                else
                begin
                  Result := jwaaclapi.SetNamedSecurityInfoA(PChar(filename),
                    JwaAccCtrl.SE_FILE_OBJECT, changedSecurityInfo, sid, groupSid, acl, nil)
                    = ERROR_SUCCESS;
                  if not Result then
                    WriteLn('Error while setting security info: ' + SysErrorMessage(getLastError()));
                  SetPrivilege(procToken, SE_RESTORE_NAME, false);
                end;
              end;
            end
        finally
          begin
            if Assigned(sid) then
              if doLocalFree then
                LocalFree(DWORD(sid))
              else
                FreeMem(sid, sidSize);
    
            // note: groupSid always needs LocalFree currently
            if Assigned(groupSid) then
                LocalFree(DWORD(groupSid));
    
            if Assigned(refDomain) then
              FreeMem(refDomain, refDomainSize);
    
            if Assigned(acl) then
              LocalFree(DWORD(acl));
          end;
        end;
    end;
    
    begin
      if not SetFilePermissionForRunAs('C:\Users\Administrator\test.txt') then
        WriteLn('Fail');
      WriteLn(Format('EXPLICIT_ACCESS_A size: %d', [SizeOf(EXPLICIT_ACCESS_A)]));
    end.
    
    
    acltest.pas (6,537 bytes)

Activities

Lukas

2019-11-27 12:51

reporter  

test.pas (217 bytes)
program test;

{$PACKENUM 4}

uses
  jwaaccctrl,
  SysUtils;

begin
  WriteLn(Format('EXPLICIT_ACCESS_A size: %d ENUM SIZE: %d', [SizeOf(jwaaccctrl.EXPLICIT_ACCESS_A), SizeOf(jwaaccctrl.ACCESS_MODE)]));
end.
test.pas (217 bytes)
acltest.pas (6,537 bytes)
program acltest;

{$MODE DELPHI}

uses
  sysutils,
  JwaWinNT,
  JwaAclApi,
  JwaAccCtrl,
  JwaSDDL,
  jwawinbase,
  jwawinsta,
  jwawintype,
  jwawinerror;

function SetFilePermissionForRunAs(filename: string): boolean;
var
  sid: JwaWinNT.PSID;
  groupSid: JwaWinNT.PSID;
  sidSize: DWORD;
  nameUse: JwaWinNT.SID_NAME_USE;
  refDomain: LpStr;
  refDomainSize: DWORD;
  dolocalfree: boolean;
  procToken: HANDLE;
  changedSecurityInfo: DWORD;
  acl: jwawinnt.PACL;

  function SetPrivilege(token: HANDLE; privilege: PChar; state: boolean): boolean;
  var
    luid: jwawintype._LUID;
    tp: jwawinnt.TOKEN_PRIVILEGES;
  begin
    if not jwawinbase.LookupPrivilegeValue(nil, privilege, luid) then
      Result := false
    else
    begin
      FillChar(tp, sizeOf(tp), #0);
      tp.privilegeCount := 1;
      tp.Privileges[0].Luid := luid;
      if state then
        tp.Privileges[0].Attributes := SE_PRIVILEGE_ENABLED
      else
        tp.Privileges[0].Attributes := SE_PRIVILEGE_REMOVED;

      Result := AdjustTokenPrivileges(token, LongBool(false), @tp, sizeOf(tp), nil,nil);
    end
  end;

  function SetupAccess(owner: jwawinnt.PSID; var acl: jwawinnt.PACL): bool;
  const
    EA_COUNT = 3;
  var
    sidAuthWorld: jwawinnt.SID_IDENTIFIER_AUTHORITY;
    sidAuthNT: jwawinnt.SID_IDENTIFIER_AUTHORITY;
    everyoneSID: jwawinnt.PSID;
    adminSID: jwawinnt.PSID;
    ea: Array[0..(EA_COUNT-1)] of jwaaccctrl.EXPLICIT_ACCESS_A;
    status: jwawintype.DWORD;
  begin
    try
      begin
        sidAuthWorld := jwawinnt.SECURITY_WORLD_SID_AUTHORITY;
        sidAuthNT := jwawinnt.SECURITY_NT_AUTHORITY;

        if not (jwawinbase.AllocateAndInitializeSid(@sidAuthWorld, 1, jwawinnt.SECURITY_WORLD_RID,
            0, 0, 0, 0, 0, 0, 0, everyoneSID)
          and AllocateAndInitializeSid(@sidAuthNT, 2, jwawinnt.SECURITY_BUILTIN_DOMAIN_RID,
          jwawinnt.DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0, adminSID)) then
        begin
          WriteLn('Could not allocate SIDs: ' + SysErrorMessage(getLastError()));
          Result := false;
        end
        else
        begin
          jwawinbase.ZeroMemory(@ea, EA_COUNT * sizeOf(EXPLICIT_ACCESS_A));

          ea[0].grfAccessPermissions := GENERIC_ALL;
          ea[0].grfAccessMode := DENY_ACCESS;
          ea[0].grfInheritance := NO_INHERITANCE;
          ea[0].Trustee.MultipleTrusteeOperation := NO_MULTIPLE_TRUSTEE;
          ea[0].Trustee.pMultipleTrustee := nil;
          ea[0].Trustee.TrusteeForm := TRUSTEE_IS_SID;
          ea[0].Trustee.TrusteeType := TRUSTEE_IS_WELL_KNOWN_GROUP;
          ea[0].Trustee.ptstrName := pointer(everyoneSID);

          ea[1].grfAccessPermissions := GENERIC_ALL;
          ea[1].grfAccessMode := SET_ACCESS;
          ea[1].grfInheritance := NO_INHERITANCE;
          ea[1].Trustee.MultipleTrusteeOperation := NO_MULTIPLE_TRUSTEE;
          ea[1].Trustee.pMultipleTrustee := nil;
          ea[1].Trustee.TrusteeForm := TRUSTEE_IS_SID;
          ea[1].Trustee.TrusteeType := TRUSTEE_IS_GROUP;
          ea[1].Trustee.ptstrName := pointer(adminSID);

          ea[2].grfAccessPermissions := GENERIC_ALL;
          ea[2].grfAccessMode := SET_ACCESS;
          ea[2].grfInheritance := NO_INHERITANCE;
          ea[2].Trustee.MultipleTrusteeOperation := NO_MULTIPLE_TRUSTEE;
          ea[2].Trustee.pMultipleTrustee := nil;
          ea[2].Trustee.TrusteeForm := TRUSTEE_IS_SID;
          ea[2].Trustee.TrusteeType := TRUSTEE_IS_USER;
          ea[2].Trustee.ptstrName := pointer(owner);

          status := jwaaclapi.SetEntriesInAclA(2, @ea, nil, acl);
          if status = ERROR_SUCCESS then
            Result := true
          else
          begin
            WriteLn('Error in SetEntriesInAcl: ' + IntToStr(status));
            Result := false;
          end;
        end;
      end
    finally
      If Assigned(everyoneSID) then
        jwawinbase.FreeSID(everyoneSID);
      If Assigned(adminSID) then
        jwawinbase.FreeSID(adminSID);
    end
  end;
begin
    try
      begin
        sidSize := 0;
        refDomainSize := 0;
        doLocalFree := false;

        procToken := INVALID_HANDLE_VALUE;

        groupSid := nil;
        sid := nil;
        changedSecurityInfo := jwawinnt.OWNER_SECURITY_INFORMATION
          or jwawinnt.DACL_SECURITY_INFORMATION;

          jwawinbase.LookupAccountNameA(nil, 'Administrator', nil,
            sidSize, nil, refDomainSize, nameUse);

          GetMem(sid, sidSize);
          GetMem(refDomain, refDomainSize);

          Result := jwawinbase.LookupAccountNameA(nil, 'Administrator',
            sid, sidSize, refDomain, refDomainSize, nameUse);

          if not SetupAccess(sid, acl) then
          begin
            Result := false;
          end
          else if jwaaclapi.SetNamedSecurityInfoA(PChar(filename),
            JwaAccCtrl.SE_FILE_OBJECT, changedSecurityInfo, sid, groupSid, acl, nil)
            <> ERROR_SUCCESS then
          begin
            // if it fails we are probably missing the SE_RESTORE_NAME privilege, so we retry
            if (not OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, procToken)) or
               (not SetPrivilege(procToken, SE_RESTORE_NAME, true)) then
              Result := false
            else
            begin
              Result := jwaaclapi.SetNamedSecurityInfoA(PChar(filename),
                JwaAccCtrl.SE_FILE_OBJECT, changedSecurityInfo, sid, groupSid, acl, nil)
                = ERROR_SUCCESS;
              if not Result then
                WriteLn('Error while setting security info: ' + SysErrorMessage(getLastError()));
              SetPrivilege(procToken, SE_RESTORE_NAME, false);
            end;
          end;
        end
    finally
      begin
        if Assigned(sid) then
          if doLocalFree then
            LocalFree(DWORD(sid))
          else
            FreeMem(sid, sidSize);

        // note: groupSid always needs LocalFree currently
        if Assigned(groupSid) then
            LocalFree(DWORD(groupSid));

        if Assigned(refDomain) then
          FreeMem(refDomain, refDomainSize);

        if Assigned(acl) then
          LocalFree(DWORD(acl));
      end;
    end;
end;

begin
  if not SetFilePermissionForRunAs('C:\Users\Administrator\test.txt') then
    WriteLn('Fail');
  WriteLn(Format('EXPLICIT_ACCESS_A size: %d', [SizeOf(EXPLICIT_ACCESS_A)]));
end.

acltest.pas (6,537 bytes)

Thaddy de Koning

2019-11-29 04:49

reporter   ~0119545

Did you report it on project Jedi?

Marco van de Voort

2019-11-29 11:00

manager   ~0119548

I did find something, please try trunk/r43608 and report back.

Lukas

2019-12-02 12:13

reporter   ~0119578

This trunk version fixes the issue for me. Below svn revision (just to be sure, was my first time actually using svn) and output of test.pas. acltest.pas also works as expected.

c:\freepascal\fpc\trunk>svn info
Path: .
Working Copy Root Path: C:\freepascal\fpc\trunk
URL: https://svn.freepascal.org/svn/fpc/trunk
Relative URL: ^/trunk
Repository Root: https://svn.freepascal.org/svn/fpc
Repository UUID: 3ad0048d-3df7-0310-abae-a5850022a9f2
Revision: 43608
Node Kind: directory
Schedule: normal
Last Changed Author: marco
Last Changed Rev: 43608
Last Changed Date: 2019-11-29 10:59:24 +0100 (Fr, 29 Nov 2019)

C:\Users\Administrator\Documents\GitHub>C:\freepascal\fpc\trunk\bin\i386-win32\fpc.exe test.pas
Free Pascal Compiler version 3.3.1 [2019/12/02] for i386
Copyright (c) 1993-2019 by Florian Klaempfl and others
Target OS: Win32 for i386
Compiling test.pas
Linking test.exe
8 lines compiled, 1.1 sec, 70096 bytes code, 4276 bytes data

C:\Users\Administrator\Documents\GitHub>test.exe
EXPLICIT_ACCESS_A size: 32 ENUM SIZE: 4

Marco van de Voort

2019-12-02 16:14

manager   ~0119589

Thank you for confirmation - > fixed

Issue History

Date Modified Username Field Change
2019-11-27 12:51 Lukas New Issue
2019-11-27 12:51 Lukas File Added: test.pas
2019-11-27 12:51 Lukas File Added: acltest.pas
2019-11-29 04:49 Thaddy de Koning Note Added: 0119545
2019-11-29 11:00 Marco van de Voort Note Added: 0119548
2019-11-29 11:43 Marco van de Voort Assigned To => Marco van de Voort
2019-11-29 11:43 Marco van de Voort Status new => feedback
2019-11-29 11:43 Marco van de Voort FPCTarget => -
2019-12-02 12:13 Lukas Note Added: 0119578
2019-12-02 12:13 Lukas Status feedback => assigned
2019-12-02 16:14 Marco van de Voort Status assigned => resolved
2019-12-02 16:14 Marco van de Voort Resolution open => fixed
2019-12-02 16:14 Marco van de Voort Fixed in Revision => 43608
2019-12-02 16:14 Marco van de Voort Note Added: 0119589