Skip to content

Commit bbc2291

Browse files
lgoldsteinlgoldstein
committed
Merge pull request #602 from lgoldstein/sid-free
Make sure SID related memory is properly released once no longer required
2 parents c76c9ff + e7809ac commit bbc2291

File tree

4 files changed

+136
-85
lines changed

4 files changed

+136
-85
lines changed

‎CHANGES.md

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ Bug Fixes
5050
* [#593](https://github.com/java-native-access/jna/pull/593): Improve binding of TypeLib bindings - [@matthiasblaesing](https://github.com/matthiasblaesing).
5151
* [#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)
5252
* [#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).
53+
* [#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)
5354

5455
Release 4.2.1
5556
=============

‎contrib/platform/src/com/sun/jna/platform/win32/Advapi32.java

+28-26
Original file line numberDiff line numberDiff line change
@@ -15,35 +15,33 @@
1515
import com.sun.jna.Native;
1616
import com.sun.jna.Pointer;
1717
import com.sun.jna.Structure;
18-
import com.sun.jna.WString;
19-
import com.sun.jna.platform.win32.WinBase.SECURITY_ATTRIBUTES;
20-
import com.sun.jna.platform.win32.WinBase.STARTUPINFO;
2118
import com.sun.jna.platform.win32.WinBase.FE_EXPORT_FUNC;
2219
import com.sun.jna.platform.win32.WinBase.FE_IMPORT_FUNC;
2320
import com.sun.jna.platform.win32.WinBase.PROCESS_INFORMATION;
21+
import com.sun.jna.platform.win32.WinBase.SECURITY_ATTRIBUTES;
22+
import com.sun.jna.platform.win32.WinBase.STARTUPINFO;
23+
import com.sun.jna.platform.win32.WinDef.BOOLByReference;
24+
import com.sun.jna.platform.win32.WinDef.DWORD;
25+
import com.sun.jna.platform.win32.WinDef.DWORDByReference;
26+
import com.sun.jna.platform.win32.WinDef.ULONG;
27+
import com.sun.jna.platform.win32.WinNT.GENERIC_MAPPING;
2428
import com.sun.jna.platform.win32.WinNT.HANDLE;
2529
import com.sun.jna.platform.win32.WinNT.HANDLEByReference;
30+
import com.sun.jna.platform.win32.WinNT.PRIVILEGE_SET;
2631
import com.sun.jna.platform.win32.WinNT.PSID;
2732
import com.sun.jna.platform.win32.WinNT.PSIDByReference;
2833
import com.sun.jna.platform.win32.WinReg.HKEY;
2934
import com.sun.jna.platform.win32.WinReg.HKEYByReference;
35+
import com.sun.jna.platform.win32.Winsvc.ChangeServiceConfig2Info;
3036
import com.sun.jna.platform.win32.Winsvc.SC_HANDLE;
3137
import com.sun.jna.platform.win32.Winsvc.SERVICE_STATUS;
3238
import com.sun.jna.platform.win32.Winsvc.SERVICE_STATUS_PROCESS;
33-
import com.sun.jna.platform.win32.Winsvc.ChangeServiceConfig2Info;
3439
import com.sun.jna.ptr.IntByReference;
3540
import com.sun.jna.ptr.LongByReference;
3641
import com.sun.jna.ptr.PointerByReference;
3742
import com.sun.jna.win32.StdCallLibrary;
3843
import com.sun.jna.win32.W32APIOptions;
3944

40-
import static com.sun.jna.platform.win32.WinDef.BOOLByReference;
41-
import static com.sun.jna.platform.win32.WinDef.DWORD;
42-
import static com.sun.jna.platform.win32.WinDef.DWORDByReference;
43-
import static com.sun.jna.platform.win32.WinDef.ULONG;
44-
import static com.sun.jna.platform.win32.WinNT.GENERIC_MAPPING;
45-
import static com.sun.jna.platform.win32.WinNT.PRIVILEGE_SET;
46-
4745
/**
4846
* Advapi32.dll Interface.
4947
*
@@ -185,28 +183,32 @@ boolean LookupAccountSid(String lpSystemName, PSID Sid,
185183
/**
186184
* Convert a security identifier (SID) to a string format suitable for
187185
* display, storage, or transmission.
188-
* http://msdn.microsoft.com/en-us/library/aa376399(VS.85).aspx
189186
*
190187
* @param Sid
191188
* The SID structure to be converted.
192189
* @param StringSid
193190
* Pointer to a variable that receives a pointer to a
194191
* null-terminated SID string. To free the returned buffer, call
195192
* the LocalFree function.
196-
* @return True if the function was successful, False otherwise.
193+
* @return {@code true} if the function was successful - call {@code GetLastError()}
194+
* to check failure reason
195+
* @see <A HREF="http://msdn.microsoft.com/en-us/library/aa376399(VS.85).aspx">ConvertSidToStringSid</A>
197196
*/
198197
boolean ConvertSidToStringSid(PSID Sid, PointerByReference StringSid);
199198

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

@@ -1231,30 +1233,30 @@ boolean ReadEventLog(HANDLE hEventLog, int dwReadFlags,
12311233
*/
12321234
public boolean ChangeServiceConfig2(SC_HANDLE hService, int dwInfoLevel,
12331235
ChangeServiceConfig2Info lpInfo);
1234-
1236+
12351237
/**
12361238
* Retrieves the optional configuration parameters of the specified service.
1237-
*
1239+
*
12381240
* @param hService
1239-
* A handle to the service. This handle is returned by the OpenService or
1240-
* CreateService function and must have the SERVICE_QUERY_CONFIG access right. For
1241+
* A handle to the service. This handle is returned by the OpenService or
1242+
* CreateService function and must have the SERVICE_QUERY_CONFIG access right. For
12411243
* more information, see Service Security and Access Rights.
12421244
* @param dwInfoLevel
12431245
* The configuration information to be queried.
12441246
* @param lpBuffer
1245-
* A pointer to the buffer that receives the service configuration information. The
1247+
* A pointer to the buffer that receives the service configuration information. The
12461248
* format of this data depends on the value of the dwInfoLevel parameter.
1247-
* The maximum size of this array is 8K bytes. To determine the required size,
1248-
* specify NULL for this parameter and 0 for the cbBufSize parameter. The function
1249-
* fails and GetLastError returns ERROR_INSUFFICIENT_BUFFER. The pcbBytesNeeded
1249+
* The maximum size of this array is 8K bytes. To determine the required size,
1250+
* specify NULL for this parameter and 0 for the cbBufSize parameter. The function
1251+
* fails and GetLastError returns ERROR_INSUFFICIENT_BUFFER. The pcbBytesNeeded
12501252
* parameter receives the needed size.
12511253
* @param cbBufSize
12521254
* The size of the structure pointed to by the lpBuffer parameter, in bytes.
12531255
* @param pcbBytesNeeded
1254-
* A pointer to a variable that receives the number of bytes required to store the
1256+
* A pointer to a variable that receives the number of bytes required to store the
12551257
* configuration information, if the function fails with ERROR_INSUFFICIENT_BUFFER.
12561258
* @return If the function succeeds, the return value is nonzero.
1257-
* If the function fails, the return value is zero. To get extended error information,
1259+
* If the function fails, the return value is zero. To get extended error information,
12581260
* call GetLastError.
12591261
*/
12601262
public boolean QueryServiceConfig2(SC_HANDLE hService, int dwInfoLevel,

‎contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java

+22-8
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,13 @@ public static String convertSidToStringSid(PSID sid) {
296296
if (!Advapi32.INSTANCE.ConvertSidToStringSid(sid, stringSid)) {
297297
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
298298
}
299-
String result = stringSid.getValue().getWideString(0);
300-
Kernel32.INSTANCE.LocalFree(stringSid.getValue());
301-
return result;
299+
300+
Pointer ptr = stringSid.getValue();
301+
try {
302+
return ptr.getWideString(0);
303+
} finally {
304+
Kernel32.INSTANCE.LocalFree(ptr);
305+
}
302306
}
303307

304308
/**
@@ -314,7 +318,13 @@ public static byte[] convertStringSidToSid(String sidString) {
314318
if (!Advapi32.INSTANCE.ConvertStringSidToSid(sidString, pSID)) {
315319
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
316320
}
317-
return pSID.getValue().getBytes();
321+
322+
PSID value = pSID.getValue();
323+
try {
324+
return value.getBytes();
325+
} finally {
326+
Kernel32.INSTANCE.LocalFree(value.getPointer());
327+
}
318328
}
319329

320330
/**
@@ -332,8 +342,13 @@ public static boolean isWellKnownSid(String sidString, int wellKnownSidType) {
332342
if (!Advapi32.INSTANCE.ConvertStringSidToSid(sidString, pSID)) {
333343
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
334344
}
335-
return Advapi32.INSTANCE.IsWellKnownSid(pSID.getValue(),
336-
wellKnownSidType);
345+
346+
PSID value = pSID.getValue();
347+
try {
348+
return Advapi32.INSTANCE.IsWellKnownSid(value, wellKnownSidType);
349+
} finally {
350+
Kernel32.INSTANCE.LocalFree(value.getPointer());
351+
}
337352
}
338353

339354
/**
@@ -372,8 +387,7 @@ public static Account getAccountBySid(String sidString) {
372387
* @return Account.
373388
*/
374389
public static Account getAccountBySid(String systemName, String sidString) {
375-
return getAccountBySid(systemName, new PSID(
376-
convertStringSidToSid(sidString)));
390+
return getAccountBySid(systemName, new PSID(convertStringSidToSid(sidString)));
377391
}
378392

379393
/**

‎contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java

+85-51
Original file line numberDiff line numberDiff line change
@@ -137,60 +137,85 @@ public void testIsValidSid() {
137137
String sidString = EVERYONE;
138138
PSIDByReference sid = new PSIDByReference();
139139
assertTrue("SID conversion failed", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
140-
assertTrue("Converted SID not valid: " + sid.getValue(), Advapi32.INSTANCE.IsValidSid(sid.getValue()));
141-
int sidLength = Advapi32.INSTANCE.GetLengthSid(sid.getValue());
142-
assertTrue(sidLength > 0);
143-
assertTrue(Advapi32.INSTANCE.IsValidSid(sid.getValue()));
140+
141+
PSID value = sid.getValue();
142+
try {
143+
assertTrue("Converted SID not valid: " + value, Advapi32.INSTANCE.IsValidSid(value));
144+
int sidLength = Advapi32.INSTANCE.GetLengthSid(value);
145+
assertTrue("Non positive sid length", sidLength > 0);
146+
assertTrue("Invalid sid", Advapi32.INSTANCE.IsValidSid(value));
147+
} finally {
148+
assertNull("Failed to release SID", Kernel32.INSTANCE.LocalFree(value.getPointer()));
149+
}
144150
}
145151

146152
public void testGetSidLength() {
147153
String sidString = EVERYONE;
148154
PSIDByReference sid = new PSIDByReference();
149155
assertTrue("SID conversion failed", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
150-
assertEquals("Wrong SID lenght", 12, Advapi32.INSTANCE.GetLengthSid(sid.getValue()));
156+
157+
PSID value = sid.getValue();
158+
try {
159+
assertEquals("Wrong SID length", 12, Advapi32.INSTANCE.GetLengthSid(value));
160+
} finally {
161+
assertNull("Failed to free SID", Kernel32.INSTANCE.LocalFree(value.getPointer()));
162+
}
151163
}
152164

153165
public void testLookupAccountSid() {
154166
// get SID bytes
155167
String sidString = EVERYONE;
156168
PSIDByReference sid = new PSIDByReference();
157-
assertTrue(Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
158-
int sidLength = Advapi32.INSTANCE.GetLengthSid(sid.getValue());
159-
assertTrue(sidLength > 0);
160-
// lookup account
161-
IntByReference cchName = new IntByReference();
162-
IntByReference cchReferencedDomainName = new IntByReference();
163-
PointerByReference peUse = new PointerByReference();
164-
assertFalse(Advapi32.INSTANCE.LookupAccountSid(null, sid.getValue(),
165-
null, cchName, null, cchReferencedDomainName, peUse));
166-
assertEquals(W32Errors.ERROR_INSUFFICIENT_BUFFER, Kernel32.INSTANCE.GetLastError());
167-
assertTrue(cchName.getValue() > 0);
168-
assertTrue(cchReferencedDomainName.getValue() > 0);
169-
char[] referencedDomainName = new char[cchReferencedDomainName.getValue()];
170-
char[] name = new char[cchName.getValue()];
171-
assertTrue(Advapi32.INSTANCE.LookupAccountSid(null, sid.getValue(),
172-
name, cchName, referencedDomainName, cchReferencedDomainName, peUse));
173-
assertEquals(5, peUse.getPointer().getInt(0)); // SidTypeWellKnownGroup
174-
String nameString = Native.toString(name);
175-
String referencedDomainNameString = Native.toString(referencedDomainName);
176-
assertTrue(nameString.length() > 0);
177-
assertEquals("Everyone", nameString);
178-
assertTrue(referencedDomainNameString.length() == 0);
179-
assertEquals(null, Kernel32.INSTANCE.LocalFree(sid.getValue().getPointer()));
169+
assertTrue("Failed to create sid", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
170+
171+
PSID value = sid.getValue();
172+
try {
173+
int sidLength = Advapi32.INSTANCE.GetLengthSid(value);
174+
assertTrue("Non-positive sid length", sidLength > 0);
175+
// lookup account
176+
IntByReference cchName = new IntByReference();
177+
IntByReference cchReferencedDomainName = new IntByReference();
178+
PointerByReference peUse = new PointerByReference();
179+
assertFalse(Advapi32.INSTANCE.LookupAccountSid(null, value,
180+
null, cchName, null, cchReferencedDomainName, peUse));
181+
assertEquals(W32Errors.ERROR_INSUFFICIENT_BUFFER, Kernel32.INSTANCE.GetLastError());
182+
assertTrue(cchName.getValue() > 0);
183+
assertTrue(cchReferencedDomainName.getValue() > 0);
184+
char[] referencedDomainName = new char[cchReferencedDomainName.getValue()];
185+
char[] name = new char[cchName.getValue()];
186+
assertTrue(Advapi32.INSTANCE.LookupAccountSid(null, value,
187+
name, cchName, referencedDomainName, cchReferencedDomainName, peUse));
188+
assertEquals(5, peUse.getPointer().getInt(0)); // SidTypeWellKnownGroup
189+
String nameString = Native.toString(name);
190+
String referencedDomainNameString = Native.toString(referencedDomainName);
191+
assertTrue(nameString.length() > 0);
192+
assertEquals("Everyone", nameString);
193+
assertTrue(referencedDomainNameString.length() == 0);
194+
} finally {
195+
assertNull("Failed to release sid", Kernel32.INSTANCE.LocalFree(value.getPointer()));
196+
}
180197
}
181198

182199
public void testConvertSid() {
183200
String sidString = EVERYONE;
184201
PSIDByReference sid = new PSIDByReference();
185-
assertTrue(Advapi32.INSTANCE.ConvertStringSidToSid(
186-
sidString, sid));
187-
PointerByReference convertedSidStringPtr = new PointerByReference();
188-
assertTrue(Advapi32.INSTANCE.ConvertSidToStringSid(
189-
sid.getValue(), convertedSidStringPtr));
190-
String convertedSidString = convertedSidStringPtr.getValue().getWideString(0);
191-
assertEquals(convertedSidString, sidString);
192-
assertEquals(null, Kernel32.INSTANCE.LocalFree(convertedSidStringPtr.getValue()));
193-
assertEquals(null, Kernel32.INSTANCE.LocalFree(sid.getValue().getPointer()));
202+
assertTrue("Failed to convert SID string", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
203+
204+
PSID value = sid.getValue();
205+
try {
206+
PointerByReference convertedSidStringPtr = new PointerByReference();
207+
assertTrue("Failed to convert SID string", Advapi32.INSTANCE.ConvertSidToStringSid(value, convertedSidStringPtr));
208+
209+
Pointer conv = convertedSidStringPtr.getValue();
210+
try {
211+
String convertedSidString = conv.getWideString(0);
212+
assertEquals("Mismatched SID string", convertedSidString, sidString);
213+
} finally {
214+
assertNull("Failed to release string value", Kernel32.INSTANCE.LocalFree(conv));
215+
}
216+
} finally {
217+
assertNull("Failed to release sid", Kernel32.INSTANCE.LocalFree(value.getPointer()));
218+
}
194219
}
195220

196221
public void testLogonUser() {
@@ -594,26 +619,35 @@ public void testRegQueryInfoKey() {
594619
public void testIsWellKnownSid() {
595620
String sidString = EVERYONE;
596621
PSIDByReference sid = new PSIDByReference();
597-
assertTrue(Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
598-
assertTrue(Advapi32.INSTANCE.IsWellKnownSid(sid.getValue(),
599-
WELL_KNOWN_SID_TYPE.WinWorldSid));
600-
assertFalse(Advapi32.INSTANCE.IsWellKnownSid(sid.getValue(),
601-
WELL_KNOWN_SID_TYPE.WinAccountAdministratorSid));
622+
assertTrue("sid conversion failed", Advapi32.INSTANCE.ConvertStringSidToSid(sidString, sid));
623+
624+
PSID value = sid.getValue();
625+
try {
626+
assertTrue("Not a world sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinWorldSid));
627+
assertFalse("Unexpected admin sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinAccountAdministratorSid));
628+
} finally {
629+
assertNull("Failed to release sid", Kernel32.INSTANCE.LocalFree(value.getPointer()));
630+
}
602631
}
603632

604633
public void testCreateWellKnownSid() {
605634
PSID pSid = new PSID(WinNT.SECURITY_MAX_SID_SIZE);
606635
IntByReference cbSid = new IntByReference(WinNT.SECURITY_MAX_SID_SIZE);
607-
assertTrue(Advapi32.INSTANCE.CreateWellKnownSid(WELL_KNOWN_SID_TYPE.WinWorldSid,
608-
null, pSid, cbSid));
609-
assertTrue(Advapi32.INSTANCE.IsWellKnownSid(pSid,
610-
WELL_KNOWN_SID_TYPE.WinWorldSid));
611-
assertTrue(cbSid.getValue() <= WinNT.SECURITY_MAX_SID_SIZE);
636+
assertTrue("Failed to create well-known SID",
637+
Advapi32.INSTANCE.CreateWellKnownSid(WELL_KNOWN_SID_TYPE.WinWorldSid, null, pSid, cbSid));
638+
assertTrue("Not recognized as well-known SID",
639+
Advapi32.INSTANCE.IsWellKnownSid(pSid, WELL_KNOWN_SID_TYPE.WinWorldSid));
640+
assertTrue("Invalid SID size", cbSid.getValue() <= WinNT.SECURITY_MAX_SID_SIZE);
612641
PointerByReference convertedSidStringPtr = new PointerByReference();
613-
assertTrue(Advapi32.INSTANCE.ConvertSidToStringSid(
614-
pSid, convertedSidStringPtr));
615-
String convertedSidString = convertedSidStringPtr.getValue().getWideString(0);
616-
assertEquals(EVERYONE, convertedSidString);
642+
assertTrue("Failed to convert SID", Advapi32.INSTANCE.ConvertSidToStringSid(pSid, convertedSidStringPtr));
643+
644+
Pointer conv = convertedSidStringPtr.getValue();
645+
try {
646+
String convertedSidString = conv.getWideString(0);
647+
assertEquals("Mismatched SID string", EVERYONE, convertedSidString);
648+
} finally {
649+
assertNull("Failed to release string", Kernel32.INSTANCE.LocalFree(conv));
650+
}
617651
}
618652

619653
public void testOpenEventLog() {

0 commit comments

Comments
 (0)