View Issue Details

IDProjectCategoryView StatusLast Update
0036368FPCPackagespublic2019-12-02 16:14
ReporterLukas Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
OSWindows 10 
Product Version3.3.1 
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

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