Skip to content

Commit a60608e

Browse files
author
Alex Menkov
committed
8334169: Long arguments of attach operation are silently truncated on Windows
Reviewed-by: sspitsyn, cjplummer
1 parent 59bf3d7 commit a60608e

File tree

2 files changed

+212
-15
lines changed

2 files changed

+212
-15
lines changed

src/jdk.attach/windows/native/libattach/VirtualMachineImpl.c

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -54,8 +54,8 @@ typedef jint (WINAPI* EnqueueOperationFunc)
5454
static HANDLE
5555
doPrivilegedOpenProcess(DWORD dwDesiredAccess, BOOL bInheritHandle, DWORD dwProcessId);
5656

57-
/* convert jstring to C string */
58-
static void jstring_to_cstring(JNIEnv* env, jstring jstr, char* cstr, int len);
57+
/* Converts jstring to C string, returns JNI_FALSE if the string has been truncated. */
58+
static jboolean jstring_to_cstring(JNIEnv* env, jstring jstr, char* cstr, size_t cstr_buf_size);
5959

6060

6161
/*
@@ -75,9 +75,9 @@ typedef struct {
7575
char jvmLib[MAX_LIBNAME_LENGTH]; /* "jvm.dll" */
7676
char func1[MAX_FUNC_LENGTH];
7777
char func2[MAX_FUNC_LENGTH];
78-
char cmd[MAX_CMD_LENGTH]; /* "load", "dump", ... */
79-
char arg[MAX_ARGS][MAX_ARG_LENGTH]; /* arguments to command */
80-
char pipename[MAX_PIPE_NAME_LENGTH];
78+
char cmd[MAX_CMD_LENGTH + 1]; /* "load", "dump", ... */
79+
char arg[MAX_ARGS][MAX_ARG_LENGTH + 1]; /* arguments to command */
80+
char pipename[MAX_PIPE_NAME_LENGTH + 1];
8181
} DataBlock;
8282

8383
/*
@@ -377,7 +377,6 @@ JNIEXPORT jint JNICALL Java_sun_tools_attach_VirtualMachineImpl_readPipe
377377
return (jint)nread;
378378
}
379379

380-
381380
/*
382381
* Class: sun_tools_attach_VirtualMachineImpl
383382
* Method: enqueue
@@ -410,7 +409,11 @@ JNIEXPORT void JNICALL Java_sun_tools_attach_VirtualMachineImpl_enqueue
410409
/*
411410
* Command and arguments
412411
*/
413-
jstring_to_cstring(env, cmd, data.cmd, MAX_CMD_LENGTH);
412+
if (!jstring_to_cstring(env, cmd, data.cmd, sizeof(data.cmd))) {
413+
JNU_ThrowByName(env, "com/sun/tools/attach/AttachOperationFailedException",
414+
"command is too long");
415+
return;
416+
}
414417
argsLen = (*env)->GetArrayLength(env, args);
415418

416419
if (argsLen > 0) {
@@ -423,7 +426,11 @@ JNIEXPORT void JNICALL Java_sun_tools_attach_VirtualMachineImpl_enqueue
423426
if (obj == NULL) {
424427
data.arg[i][0] = '\0';
425428
} else {
426-
jstring_to_cstring(env, obj, data.arg[i], MAX_ARG_LENGTH);
429+
if (!jstring_to_cstring(env, obj, data.arg[i], sizeof(data.arg[i]))) {
430+
JNU_ThrowByName(env, "com/sun/tools/attach/AttachOperationFailedException",
431+
"argument is too long");
432+
return;
433+
}
427434
}
428435
if ((*env)->ExceptionOccurred(env)) return;
429436
}
@@ -433,7 +440,11 @@ JNIEXPORT void JNICALL Java_sun_tools_attach_VirtualMachineImpl_enqueue
433440
}
434441

435442
/* pipe name */
436-
jstring_to_cstring(env, pipename, data.pipename, MAX_PIPE_NAME_LENGTH);
443+
if (!jstring_to_cstring(env, pipename, data.pipename, sizeof(data.pipename))) {
444+
JNU_ThrowByName(env, "com/sun/tools/attach/AttachOperationFailedException",
445+
"pipe name is too long");
446+
return;
447+
}
437448

438449
/*
439450
* Allocate memory in target process for data and code stub
@@ -615,21 +626,28 @@ doPrivilegedOpenProcess(DWORD dwDesiredAccess, BOOL bInheritHandle, DWORD dwProc
615626
return hProcess;
616627
}
617628

618-
/* convert jstring to C string */
619-
static void jstring_to_cstring(JNIEnv* env, jstring jstr, char* cstr, int len) {
629+
/* Converts jstring to C string, returns JNI_FALSE if the string has been truncated. */
630+
static jboolean jstring_to_cstring(JNIEnv* env, jstring jstr, char* cstr, size_t cstr_buf_size) {
620631
jboolean isCopy;
621632
const char* str;
633+
jboolean result = JNI_TRUE;
622634

623635
if (jstr == NULL) {
624636
cstr[0] = '\0';
625637
} else {
626638
str = JNU_GetStringPlatformChars(env, jstr, &isCopy);
627-
if ((*env)->ExceptionOccurred(env)) return;
639+
if ((*env)->ExceptionOccurred(env)) {
640+
return result;
641+
}
642+
if (strlen(str) >= cstr_buf_size) {
643+
result = JNI_FALSE;
644+
}
628645

629-
strncpy(cstr, str, len);
630-
cstr[len-1] = '\0';
646+
strncpy(cstr, str, cstr_buf_size);
647+
cstr[cstr_buf_size - 1] = '\0';
631648
if (isCopy) {
632649
JNU_ReleaseStringPlatformChars(env, jstr, str);
633650
}
634651
}
652+
return result;
635653
}
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @summary Tests that long arguments of attach operation are not truncated
27+
* @bug 8334168
28+
* @library /test/lib
29+
* @modules jdk.attach/sun.tools.attach
30+
* @run main LongArgTest
31+
*/
32+
33+
import java.io.BufferedReader;
34+
import java.io.InputStreamReader;
35+
import java.io.InputStream;
36+
import java.io.IOException;
37+
38+
import com.sun.tools.attach.VirtualMachine;
39+
import sun.tools.attach.HotSpotVirtualMachine;
40+
41+
import jdk.test.lib.apps.LingeredApp;
42+
43+
public class LongArgTest {
44+
45+
// current restriction: max arg size is 1024
46+
private static int MAX_ARG_SIZE = 1024;
47+
48+
public static void main(String[] args) throws Exception {
49+
LingeredApp app = null;
50+
try {
51+
app = LingeredApp.startApp();
52+
53+
// sanity
54+
test(app)
55+
.mustSucceed()
56+
.run();
57+
58+
test(app)
59+
.valueLength(MAX_ARG_SIZE)
60+
.mustSucceed()
61+
.run();
62+
63+
test(app)
64+
.valueLength(MAX_ARG_SIZE + 1)
65+
.run();
66+
67+
// more than max args (3) with MAX_ARG_SIZE
68+
test(app)
69+
.valueLength(3 * MAX_ARG_SIZE + 1)
70+
.run();
71+
72+
} finally {
73+
LingeredApp.stopApp(app);
74+
}
75+
}
76+
77+
private static Test test(LingeredApp app) {
78+
return new Test(app);
79+
}
80+
81+
// For simplicity, the test uses internal HotSpotVirtualMachine,
82+
// sets/gets "HeapDumpPath" flag value (string flag, not validated by JVM).
83+
private static class Test {
84+
private LingeredApp app;
85+
private String flagName = "HeapDumpPath";
86+
private String flagValue = generateValue(5);
87+
private boolean setFlagMustSucceed = false;
88+
89+
Test(LingeredApp app) {
90+
this.app = app;
91+
}
92+
93+
Test valueLength(int len) {
94+
flagValue = generateValue(len);
95+
return this;
96+
}
97+
98+
Test mustSucceed() {
99+
setFlagMustSucceed = true;
100+
return this;
101+
}
102+
103+
void run() throws Exception {
104+
System.out.println("======== Start ========");
105+
System.out.println("Arg size = " + flagValue.length());
106+
107+
HotSpotVirtualMachine vm = (HotSpotVirtualMachine)VirtualMachine.attach(String.valueOf(app.getPid()));
108+
109+
if (setFlag(vm)) {
110+
String actualValue = getFlag(vm);
111+
112+
if (!flagValue.equals(actualValue)) {
113+
String msg = "Actual value is different: ";
114+
if (actualValue == null) {
115+
msg += "null";
116+
} else if (flagValue.startsWith(actualValue)) {
117+
msg += "truncated from " + flagValue.length() + " to " + actualValue.length();
118+
} else {
119+
msg += actualValue + ", expected value: " + flagValue;
120+
}
121+
System.out.println(msg);
122+
vm.detach();
123+
throw new RuntimeException(msg);
124+
} else {
125+
System.out.println("Actual value matches: " + actualValue);
126+
}
127+
}
128+
129+
vm.detach();
130+
131+
System.out.println("======== End ========");
132+
System.out.println();
133+
}
134+
135+
// Sets the flag value, return true on success.
136+
private boolean setFlag(HotSpotVirtualMachine vm) throws Exception {
137+
BufferedReader replyReader = null;
138+
try {
139+
replyReader = new BufferedReader(new InputStreamReader(
140+
vm.setFlag(flagName, flagValue)));
141+
} catch (IOException ex) {
142+
if (setFlagMustSucceed) {
143+
throw ex;
144+
}
145+
System.out.println("OK: setFlag() thrown exception:");
146+
ex.printStackTrace(System.out);
147+
return false;
148+
}
149+
150+
String line;
151+
while ((line = replyReader.readLine()) != null) {
152+
System.out.println("setFlag: " + line);
153+
}
154+
replyReader.close();
155+
return true;
156+
}
157+
158+
private String getFlag(HotSpotVirtualMachine vm) throws Exception {
159+
// Then read and make sure we get back the same value.
160+
BufferedReader replyReader = new BufferedReader(new InputStreamReader(vm.printFlag(flagName)));
161+
162+
String prefix = "-XX:" + flagName + "=";
163+
String value = null;
164+
String line;
165+
while((line = replyReader.readLine()) != null) {
166+
System.out.println("getFlag: " + line);
167+
if (line.startsWith(prefix)) {
168+
value = line.substring(prefix.length());
169+
}
170+
}
171+
return value;
172+
}
173+
174+
private String generateValue(int len) {
175+
return "X" + "A".repeat(len - 2) + "X";
176+
}
177+
}
178+
179+
}

0 commit comments

Comments
 (0)