Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make sure SID related memory is properly released once no longer required #602

Merged
merged 1 commit into from
Mar 3, 2016
Merged

Make sure SID related memory is properly released once no longer required #602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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