Can not get procedure to return

[SFSxOI]

OK, I feel really stupid for asking this,,,

Problem: Can not get this to return the result string (SIDString.s). I can debug the result in the procedure, and its correct by the way, but the second I type the procedure to return the string the result is empty but the functions indicate they were sucessful so the functions aren't failing. Maybe some memory thing?

Below is the whole code i'm testing with, I have to use these functions so i can't substitute for something else. If you execute it as is you will notice the string 'SIDString' debugs fine in the procedure. The second you type the procedure for a string return (Procedure.s ....) the string is suddenly empty. In addition if you run the code as is and simply put a 'Debug' in front of the procedure call the string is empty. If you type the procedure for a string return and try to return the string it crashes at the 'FreeMemory(*secDescPtr)' line. In short the only way its working right now is without trying to return anything from it.

Code: Select all

#SDDL_REVISION_1 = 1
#DACL_SECURITY_INFORMATION = $00000004

Prototype PConvertSecurityDescriptorToStringSecurityDescriptor(SecurityDescriptor, requestedStringSDRevision, SecurityInformation, StringSecurityDescriptor, StringSecurityDescriptorLen)
Global ConvertSecurityDescriptorToStringSecurityDescriptor.PConvertSecurityDescriptorToStringSecurityDescriptor

Lib_Advapi32 = OpenLibrary(#PB_Any,"Advapi32.dll")
If IsLibrary(Lib_Advapi32) : LibAdvapi = #True
  CompilerIf #PB_Compiler_Unicode ; If compiled in unicode
    ConvertSecurityDescriptorToStringSecurityDescriptor.PConvertSecurityDescriptorToStringSecurityDescriptor=GetFunction(Lib_Advapi32,"ConvertSecurityDescriptorToStringSecurityDescriptorW")
  CompilerElse ; if not compiled in unicode
    ConvertSecurityDescriptorToStringSecurityDescriptor.PConvertSecurityDescriptorToStringSecurityDescriptor=GetFunction(Lib_Advapi32,"ConvertSecurityDescriptorToStringSecurityDescriptorA")
  CompilerEndIf
Else
  LibAdvapi = #False
EndIf

Procedure.q TopHiveKey(HiveKeyConvert$)
  Protected HiveKeyx.q
  
  If Right(HiveKeyConvert$, 1) = "\"
    HiveKeyConvert$ = RemoveString(HiveKeyConvert$, "\", #PB_String_NoCase, Len(HiveKeyConvert$) - 1, 1)
  Else
    HiveKeyConvert$ = HiveKeyConvert$
  EndIf
  
  HiveKeya$=StringField(HiveKeyConvert$,1,"\")
  HiveKeyTop$=UCase(HiveKeya$)
  Select HiveKeyTop$
    Case "HKEY_CLASSES_ROOT"
      HiveKeyx = #HKEY_CLASSES_ROOT 
    Case "HKEY_CURRENT_USER"
      HiveKeyx = #HKEY_CURRENT_USER
    Case "HKEY_LOCAL_MACHINE"
      HiveKeyx = #HKEY_LOCAL_MACHINE
    Case "HKEY_USERS"
      HiveKeyx = #HKEY_USERS
    Case "HKEY_CURRENT_CONFIG"
      HiveKeyx = #HKEY_CURRENT_CONFIG
    Default
      HiveKeyx = #Null
  EndSelect
  ProcedureReturn HiveKeyx
  
EndProcedure

Procedure.s KeyConvert(KeyToConvert$)
  
  If Right(KeyToConvert$, 1) = "\"
    KeyToConvert$ = RemoveString(KeyToConvert$, "\", #PB_String_NoCase, Len(KeyToConvert$) - 1, 1)
  Else
    KeyToConvert$ = KeyToConvert$
  EndIf
  
  GetBackSlash=FindString(KeyToConvert$,"\",1)
  KeyNameX$=Right(KeyToConvert$,(Len(KeyToConvert$)-GetBackSlash))
  If Left(KeyNameX$, 1) = "\" 
    KeyNameX$ = Right(KeyNameX$, Len(KeyNameX$) - 1) 
  EndIf
  ProcedureReturn KeyNameX$
EndProcedure

Procedure Reg_GetKeySecurity(Key.s, secInfo.i)
  Protected hKey.l, HiveKey.q, KeyName$, SIDString.s, retfunc.i
  cbLength = 0
  HiveKey = TopHiveKey(Key)
  KeyName$ = KeyConvert(Key)
  *secDescPtr.SECURITY_DESCRIPTOR = #Null
  
  FuncRet = RegOpenKeyEx_(HiveKey, @KeyName$, 0, #KEY_ALL_ACCESS|#READ_CONTROL, @hKey)
  If FuncRet = #ERROR_SUCCESS
    retfunc = RegGetKeySecurity_(hKey, @secInfo, 0, @cbLength) ; get buffer size
    If retfunc = #ERROR_INSUFFICIENT_BUFFER
      *secDescPtr = AllocateMemory(cbLength)
      retfunc = RegGetKeySecurity_(hKey, @secInfo, *secDescPtr, @cbLength)
    EndIf
  EndIf
  
  *pSID=AllocateMemory(255)
  FuncRet = ConvertSecurityDescriptorToStringSecurityDescriptor(*secDescPtr, #SDDL_REVISION_1, @secInfo, @*pSID, #Null); output in SDDL format
  memlen = MemoryStringLength(*pSID)
  SIDString=PeekS(*pSID,memlen)
  LocalFree_(*pSID)
  Debug SIDString
        
  FreeMemory(*secDescPtr)
  
  RegCloseKey_(hKey)
  ProcedureReturn ;SIDString
EndProcedure


Reg_GetKeySecurity("HKEY_CURRENT_USER\Control Panel", #DACL_SECURITY_INFORMATION)

CloseLibrary(Lib_Advapi32)

What am I missing?

(yes, thats the actual API name - ConvertSecurityDescriptorToStringSecurityDescriptor - probably the longest API name there is )

[RASHAD]

Hi

Code: Select all

#SDDL_REVISION_1 = 1
#DACL_SECURITY_INFORMATION = $00000004

Prototype PConvertSecurityDescriptorToStringSecurityDescriptor(SecurityDescriptor, requestedStringSDRevision, SecurityInformation, StringSecurityDescriptor, StringSecurityDescriptorLen)
Global ConvertSecurityDescriptorToStringSecurityDescriptor.PConvertSecurityDescriptorToStringSecurityDescriptor
Global SIDString.s

Lib_Advapi32 = OpenLibrary(#PB_Any,"Advapi32.dll")
If IsLibrary(Lib_Advapi32) : LibAdvapi = #True
  CompilerIf #PB_Compiler_Unicode ; If compiled in unicode
    ConvertSecurityDescriptorToStringSecurityDescriptor.PConvertSecurityDescriptorToStringSecurityDescriptor = GetFunction(Lib_Advapi32,"ConvertSecurityDescriptorToStringSecurityDescriptorW")
  CompilerElse ; if not compiled in unicode
    ConvertSecurityDescriptorToStringSecurityDescriptor.PConvertSecurityDescriptorToStringSecurityDescriptor = GetFunction(Lib_Advapi32,"ConvertSecurityDescriptorToStringSecurityDescriptorA")
  CompilerEndIf
Else
  LibAdvapi = #False
EndIf

Procedure.q TopHiveKey(HiveKeyConvert$)
  Protected HiveKeyx.q
 
  If Right(HiveKeyConvert$, 1) = "\"
    HiveKeyConvert$ = RemoveString(HiveKeyConvert$, "\", #PB_String_NoCase, Len(HiveKeyConvert$) - 1, 1)
  Else
    HiveKeyConvert$ = HiveKeyConvert$
  EndIf
 
  HiveKeya$=StringField(HiveKeyConvert$,1,"\")
  HiveKeyTop$=UCase(HiveKeya$)
  Select HiveKeyTop$
    Case "HKEY_CLASSES_ROOT"
      HiveKeyx = #HKEY_CLASSES_ROOT
    Case "HKEY_CURRENT_USER"
      HiveKeyx = #HKEY_CURRENT_USER
    Case "HKEY_LOCAL_MACHINE"
      HiveKeyx = #HKEY_LOCAL_MACHINE
    Case "HKEY_USERS"
      HiveKeyx = #HKEY_USERS
    Case "HKEY_CURRENT_CONFIG"
      HiveKeyx = #HKEY_CURRENT_CONFIG
    Default
      HiveKeyx = #Null
  EndSelect
  ProcedureReturn HiveKeyx
 
EndProcedure

Procedure.s KeyConvert(KeyToConvert$)
 
  If Right(KeyToConvert$, 1) = "\"
    KeyToConvert$ = RemoveString(KeyToConvert$, "\", #PB_String_NoCase, Len(KeyToConvert$) - 1, 1)
  Else
    KeyToConvert$ = KeyToConvert$
  EndIf
 
  GetBackSlash=FindString(KeyToConvert$,"\",1)
  KeyNameX$=Right(KeyToConvert$,(Len(KeyToConvert$)-GetBackSlash))
  If Left(KeyNameX$, 1) = "\"
    KeyNameX$ = Right(KeyNameX$, Len(KeyNameX$) - 1)
  EndIf
  ProcedureReturn KeyNameX$
EndProcedure

Procedure Reg_GetKeySecurity(Key.s, secInfo.i)
  Protected hKey.l, HiveKey.q, KeyName$,retfunc.i
  cbLength = 0
  HiveKey = TopHiveKey(Key)
  KeyName$ = KeyConvert(Key)
  *secDescPtr.SECURITY_DESCRIPTOR = #Null
 
  FuncRet = RegOpenKeyEx_(HiveKey, @KeyName$, 0, #KEY_ALL_ACCESS|#READ_CONTROL, @hKey)
  If FuncRet = #ERROR_SUCCESS
    retfunc = RegGetKeySecurity_(hKey, @secInfo, 0, @cbLength) ; get buffer size
    If retfunc = #ERROR_INSUFFICIENT_BUFFER
      *secDescPtr = AllocateMemory(cbLength)
      retfunc = RegGetKeySecurity_(hKey, @secInfo, *secDescPtr, @cbLength)
    EndIf
  EndIf
 
  *pSID=AllocateMemory(255)
  FuncRet = ConvertSecurityDescriptorToStringSecurityDescriptor(*secDescPtr, #SDDL_REVISION_1, @secInfo, @*pSID, #Null); output in SDDL format
  memlen = MemoryStringLength(*pSID)
  SIDString=PeekS(*pSID,memlen)
  LocalFree_(*pSID)
  Debug SIDString       
  FreeMemory(*secDescPtr) 
 
  RegCloseKey_(hKey)
  
  ProcedureReturn 0
EndProcedure


Reg_GetKeySecurity("HKEY_CURRENT_USER\Control Panel", #DACL_SECURITY_INFORMATION)
Debug SIDString

CloseLibrary(Lib_Advapi32)

[srod]

Switch all occurrences of @secInfo for just secInfo in the Reg_GetKeySecurity function.

[Demivec]

This isn't necessarily a solution but a problem exists because you aren't testing for the case of a Null pointer.

Code: Select all

  If *secDescPtr
    *pSID=AllocateMemory(255)
    FuncRet = ConvertSecurityDescriptorToStringSecurityDescriptor(*secDescPtr, #SDDL_REVISION_1, @secInfo, @*pSID, #Null); output in SDDL format
    memlen = MemoryStringLength(*pSID)
    SIDString=PeekS(*pSID,memlen)
    LocalFree_(*pSID)
    Debug SIDString
    
    FreeMemory(*secDescPtr)
  EndIf 

You also have some unnecessary code where you do things like this:

Code: Select all

HiveKeyConvert$ = HiveKeyConvert$

[luis]

Code: Select all

Procedure.s Reg_GetKeySecurity(Key.s, secInfo.i)
  Protected hKey.l, HiveKey.q, KeyName$, SIDString.s, retfunc.i
  cbLength = 0
  HiveKey = TopHiveKey(Key)
  KeyName$ = KeyConvert(Key)
  *secDescPtr.SECURITY_DESCRIPTOR = #Null
 
  FuncRet = RegOpenKeyEx_(HiveKey, @KeyName$, 0, #KEY_READ, @hKey)
  If FuncRet = #ERROR_SUCCESS
    ;;; why secinfo by address ? read the api doc
    retfunc = RegGetKeySecurity_(hKey, secInfo, *secDescPtr, @cbLength) ;;; why no destination buffer ? read the api doc
    If retfunc = #ERROR_INSUFFICIENT_BUFFER ;;; why this test ? and if fails what happen ? nothing good I can tell you
      *secDescPtr = AllocateMemory(cbLength)
      retfunc = RegGetKeySecurity_(hKey, secInfo, *secDescPtr, @cbLength)
    EndIf
  EndIf
 
  ;;; *pSID=AllocateMemory(255)
  *pSID = LocalAlloc_(#LMEM_FIXED,255)
  ;;; why no @seclen ?
  FuncRet = ConvertSecurityDescriptorToStringSecurityDescriptor(*secDescPtr, #SDDL_REVISION_1, secInfo, @*pSID, @seclen); output in SDDL format
  ;;; memlen = MemoryStringLength(*pSID)
  SIDString=PeekS(*pSID,seclen)
  LocalFree_(*pSID)
  Debug SIDString
       
  FreeMemory(*secDescPtr)
 
  RegCloseKey_(hKey)
  ProcedureReturn SIDString
EndProcedure

Anyway the logic of this code is still bad.

[SFSxOI]

i'll get all the logic and code structure stuff fixed later, its just rough test code right now. Thanks for the help folks

@luis;

One of your comments was for this:

Code: Select all

;;; why secinfo by address ? read the api doc
    retfunc = RegGetKeySecurity_(hKey, secInfo, *secDescPtr, @cbLength);;; why no destination buffer ? read the api doc 

it was originally like this:

Code: Select all

retfunc = RegGetKeySecurity_(hKey, @secInfo, 0, @cbLength) ; get buffer size

The reason why the *secDescPtr wasn't in the first line and was replaced by '0' was so I could test function failure without the *secDescPtr and its effect on reliability for getting a good buffer size, it doesn't matter if its there or not at that point because all you need from that point is the buffer size, i'm going to be changing to a fixed size buffer anyway and thats going to go away anyhow. The secinfo by address was because eventually when its done its part of something bigger and will be getting its 'secinfo' parameter by address so i had to test that as well to make sure it could get the parameter by address. I'm not permitted to use the 'LocalAlloc' function so I gotta stick with AllocateMemory there for now until its replaced by a heap function.

[SFSxOI]

This isn't necessarily a solution but a problem exists because you aren't testing for the case of a Null pointer.

Code: Select all

  If *secDescPtr
    *pSID=AllocateMemory(255)
    FuncRet = ConvertSecurityDescriptorToStringSecurityDescriptor(*secDescPtr, #SDDL_REVISION_1, @secInfo, @*pSID, #Null); output in SDDL format
    memlen = MemoryStringLength(*pSID)
    SIDString=PeekS(*pSID,memlen)
    LocalFree_(*pSID)
    Debug SIDString
    
    FreeMemory(*secDescPtr)
  EndIf 

You also have some unnecessary code where you do things like this:

Code: Select all

HiveKeyConvert$ = HiveKeyConvert$

Thanks Demives, yep wasn't testing for a null pointer, thanks for pointing that out.

The 'HiveKeyConvert$ = HiveKeyConvert$' - and - 'KeyToConvert$ = KeyToConvert$' are just really place holders right now, those areas are going to be something else later.

[luis]

The reason why the *secDescPtr wasn't in the first line and was replaced by '0' was so I could test function failure without the *secDescPtr and its effect on reliability for getting a good buffer size, it doesn't matter if its there or not at that point because all you need from that point is the buffer size,

You are right.
I understand the reason behind the 0 at the location pointed by the last param (to get the size of the buffer), but I missed at the first read of the docs the fact the third param is specified as optional (and so 0).
I know it *should* be optional if the last param point to a 0, but I thought it was not documented as such and I try to not assume unless I'm forced to do so. So I put the (unused) buffer there anyway.
But the point is that comment was wrong.

But the test just below it (if retfunc = #ERROR_INSUFFICIENT_BUFFER) doesn't make much sense. If you know the call will fail (because you didn't pass the buffer, since you are asking for its size) there is no point in testing for that.
If you want to put that anyway you need at least a meaningful "else" clause. Without it, if for some miracle the above test is not true, the code inside it's not executed and later you try to free a pointer not allocated (that's what was happening with your original code).

The secinfo by address was because eventually when its done its part of something bigger and will be getting its 'secinfo' parameter by address so i had to test that as well to make sure it could get the parameter by address.

Uh ? It can't. The parameter must be by value.

LONG WINAPI RegGetKeySecurity(
__in HKEY hKey,
__in SECURITY_INFORMATION SecurityInformation,
__out_opt PSECURITY_DESCRIPTOR pSecurityDescriptor,
__inout LPDWORD lpcbSecurityDescriptor
);

typedef DWORD SECURITY_INFORMATION, *PSECURITY_INFORMATION;

SecurityInformation is a double word (or a long, or 32 bits).

So you cannot pass it as @secinfo, or you will pass a random address value instead of the secinfo value.


BTW: #KEY_ALL_ACCESS|#READ_CONTROL is redundant.

#KEY_ALL_ACCESS includes #STANDARD_RIGHTS_REQUIRED and #STANDARD_RIGHTS_REQUIRED combines #DELETE, #READ_CONTROL, #WRITE_DAC and #WRITE_OWNER.

Code: Select all

Debug #KEY_ALL_ACCESS 
Debug #KEY_ALL_ACCESS|#READ_CONTROL
[12:37:40] 983103
[12:37:40] 983103

[SFSxOI]

I needed to test that sceinario, its going to be replaced with a fixed buffer size anyway so there would be no need for it in the final. You are correct about the access, thanks for pointing that out.