Skip to content

Commit 29bc09d

Browse files
authored
Merge pull request #143 from benhumphreys/harden-null-injection
Add a check for NUL characters in NuProcessBuilder to ensure the arguments provided to the command in code are interpreted consistently when the native process is started.
2 parents 4953d61 + 467b28a commit 29bc09d

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

Diff for: src/main/java/com/zaxxer/nuprocess/NuProcessBuilder.java

+10
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ public void setCwd(Path cwd)
253253
*/
254254
public NuProcess start()
255255
{
256+
ensureNoNullCharacters(command);
256257
ensureListener();
257258
String[] env = prepareEnvironment();
258259

@@ -267,6 +268,7 @@ public NuProcess start()
267268
*/
268269
public void run()
269270
{
271+
ensureNoNullCharacters(command);
270272
ensureListener();
271273
String[] env = prepareEnvironment();
272274

@@ -280,6 +282,14 @@ private void ensureListener()
280282
}
281283
}
282284

285+
private void ensureNoNullCharacters(List<String> commands) {
286+
for (String command : commands) {
287+
if (command.indexOf('\u0000') >= 0) {
288+
throw new IllegalArgumentException("Commands may not contain null characters");
289+
}
290+
}
291+
}
292+
283293
private String[] prepareEnvironment()
284294
{
285295
String[] env = new String[environment.size()];

Diff for: src/test/java/com/zaxxer/nuprocess/RunTest.java

+29
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,32 @@ public void softCloseStdinAfterWrite()
316316
System.err.println("Completed test softCloseStdinAfterWrite()");
317317
}
318318

319+
@Test(expected = IllegalArgumentException.class)
320+
public void nullCommandViaCommandMutationWithRun() {
321+
NuProcessBuilder pb = new NuProcessBuilder(new NullProcessHandler(), command);
322+
pb.command().add("--foo\0--bar");
323+
pb.run();
324+
}
325+
326+
@Test(expected = IllegalArgumentException.class)
327+
public void nullCommandViaCommandMutationWithStart() {
328+
NuProcessBuilder pb = new NuProcessBuilder(new NullProcessHandler(), command);
329+
pb.command().add("--foo\0--bar");
330+
pb.start();
331+
}
332+
333+
@Test(expected = IllegalArgumentException.class)
334+
public void nullCommandViaConstructorWithRun() {
335+
NuProcessBuilder pb = new NuProcessBuilder(new NullProcessHandler(), command, "--foo\0--bar");
336+
pb.run();
337+
}
338+
339+
@Test(expected = IllegalArgumentException.class)
340+
public void nullCommandViaConstructorWithStart() {
341+
NuProcessBuilder pb = new NuProcessBuilder(new NullProcessHandler(), command, "--foo\0--bar");
342+
pb.start();
343+
}
344+
319345
private static byte[] getLotsOfBytes()
320346
{
321347
StringBuilder sb = new StringBuilder();
@@ -395,6 +421,9 @@ boolean checkAdlers()
395421
}
396422
}
397423

424+
private static class NullProcessHandler extends NuAbstractProcessHandler {
425+
}
426+
398427
private static class Utf8DecodingListener extends NuAbstractCharsetHandler
399428
{
400429
private final CharBuffer utf8Buffer;

0 commit comments

Comments
 (0)