Skip to content

Commit a0d8eb9

Browse files
committed
TEZ-4699:Add Validation/Canonical checks to avoid path exploitation in CSVResult.java
1 parent fd03af6 commit a0d8eb9

2 files changed

Lines changed: 145 additions & 24 deletions

File tree

tez-tools/analyzers/job-analyzer/src/main/java/org/apache/tez/analyzer/CSVResult.java

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020

2121
import java.io.BufferedWriter;
2222
import java.io.File;
23-
import java.io.FileOutputStream;
2423
import java.io.IOException;
2524
import java.io.OutputStreamWriter;
26-
import java.nio.charset.Charset;
25+
import java.nio.charset.StandardCharsets;
26+
import java.nio.file.Files;
27+
import java.nio.file.Path;
28+
import java.nio.file.StandardOpenOption;
2729
import java.util.Arrays;
2830
import java.util.Collections;
2931
import java.util.Comparator;
@@ -93,31 +95,44 @@ public void setComments(String comments) {
9395
'}';
9496
}
9597

96-
//For testing
9798
public void dumpToFile(String fileName) throws IOException {
98-
OutputStreamWriter writer = new OutputStreamWriter(
99-
new FileOutputStream(new File(fileName)),
100-
Charset.forName("UTF-8").newEncoder());
101-
BufferedWriter bw = new BufferedWriter(writer);
102-
bw.write(Joiner.on(",").join(headers));
103-
bw.newLine();
104-
for (String[] record : recordsList) {
105-
106-
if (record.length != headers.length) {
107-
continue; //LOG error msg?
108-
}
109-
110-
StringBuilder sb = new StringBuilder();
111-
for(int i=0;i<record.length;i++) {
112-
sb.append(!Strings.isNullOrEmpty(record[i]) ? record[i] : " ");
113-
if (i < record.length - 1) {
114-
sb.append(",");
99+
File target = validateOutputFile(fileName);
100+
Path targetPath = target.toPath();
101+
102+
try (BufferedWriter bw = new BufferedWriter(
103+
new OutputStreamWriter(
104+
Files.newOutputStream(targetPath, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE),
105+
StandardCharsets.UTF_8))) {
106+
bw.write(Joiner.on(",").join(headers));
107+
bw.newLine();
108+
for (String[] record : recordsList) {
109+
if (record.length != headers.length) {
110+
continue;
115111
}
112+
StringBuilder sb = new StringBuilder();
113+
for (int i = 0; i < record.length; i++) {
114+
sb.append(!Strings.isNullOrEmpty(record[i]) ? record[i] : " ");
115+
if (i < record.length - 1) {
116+
sb.append(",");
117+
}
118+
}
119+
bw.write(sb.toString());
120+
bw.newLine();
116121
}
117-
bw.write(sb.toString());
118-
bw.newLine();
119122
}
120-
bw.flush();
121-
bw.close();
123+
}
124+
125+
private static File validateOutputFile(String fileName) throws IOException {
126+
if (fileName == null || fileName.trim().isEmpty()) {
127+
throw new IllegalArgumentException("fileName must not be null or empty");
128+
}
129+
130+
File target = new File(fileName).getCanonicalFile();
131+
File parent = target.getParentFile();
132+
133+
if (parent == null || !parent.isDirectory()) {
134+
throw new IOException("Parent directory does not exist: " + parent);
135+
}
136+
return target;
122137
}
123138
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
* <p/>
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
* <p/>
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.tez.analyzer;
20+
21+
22+
import java.io.IOException;
23+
import java.nio.charset.StandardCharsets;
24+
import java.nio.file.FileAlreadyExistsException;
25+
import java.nio.file.Files;
26+
import java.nio.file.Path;
27+
28+
import org.junit.Assert;
29+
import org.junit.Rule;
30+
import org.junit.Test;
31+
import org.junit.rules.TemporaryFolder;
32+
import org.slf4j.Logger;
33+
import org.slf4j.LoggerFactory;
34+
35+
/**
36+
* Unit tests for {@link CSVResult#dumpToFile(String)} (validation and atomic create).
37+
*/
38+
public class TestCSVResult {
39+
40+
@Rule
41+
public TemporaryFolder tmpDir = new TemporaryFolder();
42+
private static final Logger LOG = LoggerFactory.getLogger(TestCSVResult.class);
43+
44+
@Test
45+
public void testDumpToFileWritesContent() throws Exception {
46+
Path out = tmpDir.newFolder().toPath().resolve("out.csv");
47+
CSVResult result = new CSVResult(new String[] { "h1", "h2" });
48+
result.addRecord(new String[] { "a", "b" });
49+
result.dumpToFile(out.toString());
50+
LOG.info("output file path is : {}", out.getFileName() );
51+
String content = new String(Files.readAllBytes(out), StandardCharsets.UTF_8);
52+
Assert.assertEquals("h1,h2\na,b\n", content);
53+
}
54+
55+
@Test
56+
public void testDumpToFileRejectsExistingFile() throws Exception {
57+
Path out = tmpDir.newFile("existing.csv").toPath();
58+
CSVResult result = new CSVResult(new String[] { "x" });
59+
try {
60+
result.dumpToFile(out.toString());
61+
Assert.fail("Expected FileAlreadyExistsException when output file already exists");
62+
} catch (FileAlreadyExistsException e) {
63+
// CREATE_NEW must fail if path already exists (atomic exclusive create)
64+
}
65+
}
66+
67+
@Test
68+
public void testDumpToFileRejectsMissingParentDir() throws Exception {
69+
Path missingParent = tmpDir.getRoot().toPath().resolve("no-such-dir");
70+
Path out = missingParent.resolve("out.csv");
71+
CSVResult result = new CSVResult(new String[] { "x" });
72+
try {
73+
result.dumpToFile(out.toString());
74+
Assert.fail("Expected IOException when parent directory does not exist");
75+
} catch (IOException e) {
76+
Assert.assertTrue(e.getMessage().contains("Parent directory does not exist"));
77+
}
78+
}
79+
80+
@Test(expected = IllegalArgumentException.class)
81+
public void testDumpToFileRejectsNullFileName() throws Exception {
82+
new CSVResult(new String[] { "x" }).dumpToFile(null);
83+
}
84+
85+
@Test(expected = IllegalArgumentException.class)
86+
public void testDumpToFileRejectsEmptyFileName() throws Exception {
87+
new CSVResult(new String[] { "x" }).dumpToFile("");
88+
}
89+
90+
@Test(expected = IllegalArgumentException.class)
91+
public void testDumpToFileRejectsBlankFileName() throws Exception {
92+
new CSVResult(new String[] { "x" }).dumpToFile(" ");
93+
}
94+
95+
@Test
96+
public void testDumpToFileNestedDirectory() throws Exception {
97+
Path base = tmpDir.newFolder().toPath();
98+
Path nested = base.resolve("a").resolve("b");
99+
Files.createDirectories(nested);
100+
Path out = nested.resolve("nested.csv");
101+
CSVResult result = new CSVResult(new String[] { "h" });
102+
result.addRecord(new String[] { "r" });
103+
result.dumpToFile(out.toString());
104+
Assert.assertEquals("h\nr\n", new String(Files.readAllBytes(out), StandardCharsets.UTF_8));
105+
}
106+
}

0 commit comments

Comments
 (0)