Skip to content

Commit

Permalink
Make sure SID related memory is properly released once no longer requ…
Browse files Browse the repository at this point in the history 
…ired
  • Loading branch information
Lyor Goldstein committed Mar 1, 2016
1 parent c76c9ff commit e7809ac
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 85 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Bug Fixes
* [#593](https://github.com/java-native-access/jna/pull/593): Improve binding of TypeLib bindings - [@matthiasblaesing](https://github.com/matthiasblaesing).
* [#578](https://github.com/java-native-access/jna/pull/578): Fix COM CallbackHandlers, allow usage of VARIANTs directly in c.s.j.p.w.COM.util.ProxyObject and fix native memory leak in c.s.j.p.w.COM.util.ProxyObject - [@matthiasblaesing](https://github.com/matthiasblaesing)
* [#601](https://github.com/java-native-access/jna/pull/601): Remove COMThread and COM initialization from objects and require callers to initialize COM themselves. Asserts are added to guard correct usage. - [@matthiasblaesing](https://github.com/matthiasblaesing).
* [#602] https://github.com/java-native-access/jna/pull/602): Make sure SID related memory is properly released once no longer required [@lgoldstein](https://github.com/lgoldstein)

Release 4.2.1
=============
Expand Down
54 changes: 28 additions & 26 deletions contrib/platform/src/com/sun/jna/platform/win32/Advapi32.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,33 @@
import com.sun.jna.Native;
import com.sun.jna.Pointer;
import com.sun.jna.Structure;
import com.sun.jna.WString;
import com.sun.jna.platform.win32.WinBase.SECURITY_ATTRIBUTES;
import com.sun.jna.platform.win32.WinBase.STARTUPINFO;
import com.sun.jna.platform.win32.WinBase.FE_EXPORT_FUNC;
import com.sun.jna.platform.win32.WinBase.FE_IMPORT_FUNC;
import com.sun.jna.platform.win32.WinBase.PROCESS_INFORMATION;
import com.sun.jna.platform.win32.WinBase.SECURITY_ATTRIBUTES;
import com.sun.jna.platform.win32.WinBase.STARTUPINFO;
import com.sun.jna.platform.win32.WinDef.BOOLByReference;
import com.sun.jna.platform.win32.WinDef.DWORD;
import com.sun.jna.platform.win32.WinDef.DWORDByReference;
import com.sun.jna.platform.win32.WinDef.ULONG;
import com.sun.jna.platform.win32.WinNT.GENERIC_MAPPING;
import com.sun.jna.platform.win32.WinNT.HANDLE;
import com.sun.jna.platform.win32.WinNT.HANDLEByReference;
import com.sun.jna.platform.win32.WinNT.PRIVILEGE_SET;
import com.sun.jna.platform.win32.WinNT.PSID;
import com.sun.jna.platform.win32.WinNT.PSIDByReference;
import com.sun.jna.platform.win32.WinReg.HKEY;
import com.sun.jna.platform.win32.WinReg.HKEYByReference;
import com.sun.jna.platform.win32.Winsvc.ChangeServiceConfig2Info;
import com.sun.jna.platform.win32.Winsvc.SC_HANDLE;
import com.sun.jna.platform.win32.Winsvc.SERVICE_STATUS;
import com.sun.jna.platform.win32.Winsvc.SERVICE_STATUS_PROCESS;
import com.sun.jna.platform.win32.Winsvc.ChangeServiceConfig2Info;
import com.sun.jna.ptr.IntByReference;
import com.sun.jna.ptr.LongByReference;
import com.sun.jna.ptr.PointerByReference;
import com.sun.jna.win32.StdCallLibrary;
import com.sun.jna.win32.W32APIOptions;

import static com.sun.jna.platform.win32.WinDef.BOOLByReference;
import static com.sun.jna.platform.win32.WinDef.DWORD;
import static com.sun.jna.platform.win32.WinDef.DWORDByReference;
import static com.sun.jna.platform.win32.WinDef.ULONG;
import static com.sun.jna.platform.win32.WinNT.GENERIC_MAPPING;
import static com.sun.jna.platform.win32.WinNT.PRIVILEGE_SET;

/**
* Advapi32.dll Interface.
*
Expand Down Expand Up @@ -185,28 +183,32 @@ boolean LookupAccountSid(String lpSystemName, PSID Sid,
/**
* Convert a security identifier (SID) to a string format suitable for
* display, storage, or transmission.
* http://msdn.microsoft.com/en-us/library/aa376399(VS.85).aspx
*
* @param Sid
* The SID structure to be converted.
* @param StringSid
* Pointer to a variable that receives a pointer to a
* null-terminated SID string. To free the returned buffer, call
* the LocalFree function.
* @return True if the function was successful, False otherwise.
* @return {@code true} if the function was successful - call {@code GetLastError()}
* to check failure reason
* @see <A HREF="http://msdn.microsoft.com/en-us/library/aa376399(VS.85).aspx">ConvertSidToStringSid</A>
*/
boolean ConvertSidToStringSid(PSID Sid, PointerByReference StringSid);

/**
* Convert a string-format security identifier (SID) into a valid,
* functional SID.
* http://msdn.microsoft.com/en-us/library/aa376402(VS.85).aspx
*
*
* @param StringSid
* The string-format SID to convert.
* @param Sid
* Receives a pointer to the converted SID.
* @return True if the function was successful, False otherwise.
* Receives a pointer to the converted SID. To free the returned buffer, call
* the LocalFree function.
* @return {@code true} if the function was successful - call {@code GetLastError()}
* to check failure reason
* @see <A HREF="http://msdn.microsoft.com/en-us/library/aa376402(VS.85).aspx">ConvertStringSidToSid</A>
*/
boolean ConvertStringSidToSid(String StringSid, PSIDByReference Sid);

Expand Down Expand Up @@ -1231,30 +1233,30 @@ boolean ReadEventLog(HANDLE hEventLog, int dwReadFlags,
*/
public boolean ChangeServiceConfig2(SC_HANDLE hService, int dwInfoLevel,
ChangeServiceConfig2Info lpInfo);

/**
* Retrieves the optional configuration parameters of the specified service.
*
*
* @param hService
* A handle to the service. This handle is returned by the OpenService or
* CreateService function and must have the SERVICE_QUERY_CONFIG access right. For
* A handle to the service. This handle is returned by the OpenService or
* CreateService function and must have the SERVICE_QUERY_CONFIG access right. For
* more information, see Service Security and Access Rights.
* @param dwInfoLevel
* The configuration information to be queried.
* @param lpBuffer
* A pointer to the buffer that receives the service configuration information. The
* A pointer to the buffer that receives the service configuration information. The
* format of this data depends on the value of the dwInfoLevel parameter.
* The maximum size of this array is 8K bytes. To determine the required size,
* specify NULL for this parameter and 0 for the cbBufSize parameter. The function
* fails and GetLastError returns ERROR_INSUFFICIENT_BUFFER. The pcbBytesNeeded
* The maximum size of this array is 8K bytes. To determine the required size,
* specify NULL for this parameter and 0 for the cbBufSize parameter. The function
* fails and GetLastError returns ERROR_INSUFFICIENT_BUFFER. The pcbBytesNeeded
* parameter receives the needed size.
* @param cbBufSize
* The size of the structure pointed to by the lpBuffer parameter, in bytes.
* @param pcbBytesNeeded
* A pointer to a variable that receives the number of bytes required to store the
* A pointer to a variable that receives the number of bytes required to store the
* configuration information, if the function fails with ERROR_INSUFFICIENT_BUFFER.
* @return If the function succeeds, the return value is nonzero.
* If the function fails, the return value is zero. To get extended error information,
* If the function fails, the return value is zero. To get extended error information,
* call GetLastError.
*/
public boolean QueryServiceConfig2(SC_HANDLE hService, int dwInfoLevel,
Expand Down
30 changes: 22 additions & 8 deletions contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,13 @@ public static String convertSidToStringSid(PSID sid) {
if (!Advapi32.INSTANCE.ConvertSidToStringSid(sid, stringSid)) {
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
}
String result = stringSid.getValue().getWideString(0);
Kernel32.INSTANCE.LocalFree(stringSid.getValue());
return result;

Pointer ptr = stringSid.getValue();
try {
return ptr.getWideString(0);
} finally {
Kernel32.INSTANCE.LocalFree(ptr);
}
}

/**
Expand All @@ -314,7 +318,13 @@ public static byte[] convertStringSidToSid(String sidString) {
if (!Advapi32.INSTANCE.ConvertStringSidToSid(sidString, pSID)) {
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
}
return pSID.getValue().getBytes();

PSID value = pSID.getValue();
try {
return value.getBytes();
} finally {
Kernel32.INSTANCE.LocalFree(value.getPointer());
}
}

/**
Expand All @@ -332,8 +342,13 @@ public static boolean isWellKnownSid(String sidString, int wellKnownSidType) {
if (!Advapi32.INSTANCE.ConvertStringSidToSid(sidString, pSID)) {
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
}
return Advapi32.INSTANCE.IsWellKnownSid(pSID.getValue(),
wellKnownSidType);

PSID value = pSID.getValue();
try {
return Advapi32.INSTANCE.IsWellKnownSid(value, wellKnownSidType);
} finally {
Kernel32.INSTANCE.LocalFree(value.getPointer());
}
}

/**
Expand Down Expand Up @@ -372,8 +387,7 @@ public static Account getAccountBySid(String sidString) {
* @return Account.
*/
public static Account getAccountBySid(String systemName, String sidString) {
return getAccountBySid(systemName, new PSID(
convertStringSidToSid(sidString)));
return getAccountBySid(systemName, new PSID(convertStringSidToSid(sidString)));
}

/**
Expand Down
136 changes: 85 additions & 51 deletions contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,60 +137,85 @@ public void testIsValidSid() {
String sidString = EVERYONE;
PSIDByReference sid = new PSIDByReference();
assertTrue("SID conversion failed", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
assertTrue("Converted SID not valid: " + sid.getValue(), Advapi32.INSTANCE.IsValidSid(sid.getValue()));
int sidLength = Advapi32.INSTANCE.GetLengthSid(sid.getValue());
assertTrue(sidLength > 0);
assertTrue(Advapi32.INSTANCE.IsValidSid(sid.getValue()));

PSID value = sid.getValue();
try {
assertTrue("Converted SID not valid: " + value, Advapi32.INSTANCE.IsValidSid(value));
int sidLength = Advapi32.INSTANCE.GetLengthSid(value);
assertTrue("Non positive sid length", sidLength > 0);
assertTrue("Invalid sid", Advapi32.INSTANCE.IsValidSid(value));
} finally {
assertNull("Failed to release SID", Kernel32.INSTANCE.LocalFree(value.getPointer()));
}
}

public void testGetSidLength() {
String sidString = EVERYONE;
PSIDByReference sid = new PSIDByReference();
assertTrue("SID conversion failed", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
assertEquals("Wrong SID lenght", 12, Advapi32.INSTANCE.GetLengthSid(sid.getValue()));

PSID value = sid.getValue();
try {
assertEquals("Wrong SID length", 12, Advapi32.INSTANCE.GetLengthSid(value));
} finally {
assertNull("Failed to free SID", Kernel32.INSTANCE.LocalFree(value.getPointer()));
}
}

public void testLookupAccountSid() {
// get SID bytes
String sidString = EVERYONE;
PSIDByReference sid = new PSIDByReference();
assertTrue(Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
int sidLength = Advapi32.INSTANCE.GetLengthSid(sid.getValue());
assertTrue(sidLength > 0);
// lookup account
IntByReference cchName = new IntByReference();
IntByReference cchReferencedDomainName = new IntByReference();
PointerByReference peUse = new PointerByReference();
assertFalse(Advapi32.INSTANCE.LookupAccountSid(null, sid.getValue(),
null, cchName, null, cchReferencedDomainName, peUse));
assertEquals(W32Errors.ERROR_INSUFFICIENT_BUFFER, Kernel32.INSTANCE.GetLastError());
assertTrue(cchName.getValue() > 0);
assertTrue(cchReferencedDomainName.getValue() > 0);
char[] referencedDomainName = new char[cchReferencedDomainName.getValue()];
char[] name = new char[cchName.getValue()];
assertTrue(Advapi32.INSTANCE.LookupAccountSid(null, sid.getValue(),
name, cchName, referencedDomainName, cchReferencedDomainName, peUse));
assertEquals(5, peUse.getPointer().getInt(0)); // SidTypeWellKnownGroup
String nameString = Native.toString(name);
String referencedDomainNameString = Native.toString(referencedDomainName);
assertTrue(nameString.length() > 0);
assertEquals("Everyone", nameString);
assertTrue(referencedDomainNameString.length() == 0);
assertEquals(null, Kernel32.INSTANCE.LocalFree(sid.getValue().getPointer()));
assertTrue("Failed to create sid", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));

PSID value = sid.getValue();
try {
int sidLength = Advapi32.INSTANCE.GetLengthSid(value);
assertTrue("Non-positive sid length", sidLength > 0);
// lookup account
IntByReference cchName = new IntByReference();
IntByReference cchReferencedDomainName = new IntByReference();
PointerByReference peUse = new PointerByReference();
assertFalse(Advapi32.INSTANCE.LookupAccountSid(null, value,
null, cchName, null, cchReferencedDomainName, peUse));
assertEquals(W32Errors.ERROR_INSUFFICIENT_BUFFER, Kernel32.INSTANCE.GetLastError());
assertTrue(cchName.getValue() > 0);
assertTrue(cchReferencedDomainName.getValue() > 0);
char[] referencedDomainName = new char[cchReferencedDomainName.getValue()];
char[] name = new char[cchName.getValue()];
assertTrue(Advapi32.INSTANCE.LookupAccountSid(null, value,
name, cchName, referencedDomainName, cchReferencedDomainName, peUse));
assertEquals(5, peUse.getPointer().getInt(0)); // SidTypeWellKnownGroup
String nameString = Native.toString(name);
String referencedDomainNameString = Native.toString(referencedDomainName);
assertTrue(nameString.length() > 0);
assertEquals("Everyone", nameString);
assertTrue(referencedDomainNameString.length() == 0);
} finally {
assertNull("Failed to release sid", Kernel32.INSTANCE.LocalFree(value.getPointer()));
}
}

public void testConvertSid() {
String sidString = EVERYONE;
PSIDByReference sid = new PSIDByReference();
assertTrue(Advapi32.INSTANCE.ConvertStringSidToSid(
sidString, sid));
PointerByReference convertedSidStringPtr = new PointerByReference();
assertTrue(Advapi32.INSTANCE.ConvertSidToStringSid(
sid.getValue(), convertedSidStringPtr));
String convertedSidString = convertedSidStringPtr.getValue().getWideString(0);
assertEquals(convertedSidString, sidString);
assertEquals(null, Kernel32.INSTANCE.LocalFree(convertedSidStringPtr.getValue()));
assertEquals(null, Kernel32.INSTANCE.LocalFree(sid.getValue().getPointer()));
assertTrue("Failed to convert SID string", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));

PSID value = sid.getValue();
try {
PointerByReference convertedSidStringPtr = new PointerByReference();
assertTrue("Failed to convert SID string", Advapi32.INSTANCE.ConvertSidToStringSid(value, convertedSidStringPtr));

Pointer conv = convertedSidStringPtr.getValue();
try {
String convertedSidString = conv.getWideString(0);
assertEquals("Mismatched SID string", convertedSidString, sidString);
} finally {
assertNull("Failed to release string value", Kernel32.INSTANCE.LocalFree(conv));
}
} finally {
assertNull("Failed to release sid", Kernel32.INSTANCE.LocalFree(value.getPointer()));
}
}

public void testLogonUser() {
Expand Down Expand Up @@ -594,26 +619,35 @@ public void testRegQueryInfoKey() {
public void testIsWellKnownSid() {
String sidString = EVERYONE;
PSIDByReference sid = new PSIDByReference();
assertTrue(Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
assertTrue(Advapi32.INSTANCE.IsWellKnownSid(sid.getValue(),
WELL_KNOWN_SID_TYPE.WinWorldSid));
assertFalse(Advapi32.INSTANCE.IsWellKnownSid(sid.getValue(),
WELL_KNOWN_SID_TYPE.WinAccountAdministratorSid));
assertTrue("sid conversion failed", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));

PSID value = sid.getValue();
try {
assertTrue("Not a world sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinWorldSid));
assertFalse("Unexpected admin sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinAccountAdministratorSid));
} finally {
assertNull("Failed to release sid", Kernel32.INSTANCE.LocalFree(value.getPointer()));
}
}

public void testCreateWellKnownSid() {
PSID pSid = new PSID(WinNT.SECURITY_MAX_SID_SIZE);
IntByReference cbSid = new IntByReference(WinNT.SECURITY_MAX_SID_SIZE);
assertTrue(Advapi32.INSTANCE.CreateWellKnownSid(WELL_KNOWN_SID_TYPE.WinWorldSid,
null, pSid, cbSid));
assertTrue(Advapi32.INSTANCE.IsWellKnownSid(pSid,
WELL_KNOWN_SID_TYPE.WinWorldSid));
assertTrue(cbSid.getValue() <= WinNT.SECURITY_MAX_SID_SIZE);
assertTrue("Failed to create well-known SID",
Advapi32.INSTANCE.CreateWellKnownSid(WELL_KNOWN_SID_TYPE.WinWorldSid, null, pSid, cbSid));
assertTrue("Not recognized as well-known SID",
Advapi32.INSTANCE.IsWellKnownSid(pSid, WELL_KNOWN_SID_TYPE.WinWorldSid));
assertTrue("Invalid SID size", cbSid.getValue() <= WinNT.SECURITY_MAX_SID_SIZE);
PointerByReference convertedSidStringPtr = new PointerByReference();
assertTrue(Advapi32.INSTANCE.ConvertSidToStringSid(
pSid, convertedSidStringPtr));
String convertedSidString = convertedSidStringPtr.getValue().getWideString(0);
assertEquals(EVERYONE, convertedSidString);
assertTrue("Failed to convert SID", Advapi32.INSTANCE.ConvertSidToStringSid(pSid, convertedSidStringPtr));

Pointer conv = convertedSidStringPtr.getValue();
try {
String convertedSidString = conv.getWideString(0);
assertEquals("Mismatched SID string", EVERYONE, convertedSidString);
} finally {
assertNull("Failed to release string", Kernel32.INSTANCE.LocalFree(conv));
}
}

public void testOpenEventLog() {
Expand Down

0 comments on commit e7809ac

Please sign in to comment.