Skip to content

Commit 49afaf2

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

1 file changed

Lines changed: 44 additions & 22 deletions

File tree

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

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

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import java.io.FileOutputStream;
2424
import java.io.IOException;
2525
import java.io.OutputStreamWriter;
26-
import java.nio.charset.Charset;
26+
import java.nio.charset.StandardCharsets;
2727
import java.util.Arrays;
2828
import java.util.Collections;
2929
import java.util.Comparator;
@@ -93,31 +93,53 @@ public void setComments(String comments) {
9393
'}';
9494
}
9595

96-
//For testing
96+
private static File validateOutputFile(String fileName) throws IOException {
97+
//null or empty string check
98+
if (fileName == null || fileName.trim().isEmpty()) {
99+
throw new IllegalArgumentException("fileName must not be null or empty");
100+
}
101+
102+
// support use of relative path
103+
File target = new File(fileName).getCanonicalFile();
104+
File parent = target.getParentFile();
105+
106+
107+
// check if the parent directory exists
108+
if (parent == null || !parent.isDirectory()) {
109+
throw new IOException("Parent directory does not exist: " + parent);
110+
}
111+
// check if the parent directory is writable
112+
if (!parent.canWrite()) {
113+
throw new IOException("No write permission on directory: " + parent);
114+
}
115+
// it should not write to any existing file to avoid file corruption
116+
if (target.exists()) {
117+
throw new IOException("Output file already exists, will not overwrite: " + target);
118+
}
119+
return target;
120+
}
121+
97122
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-
}
123+
File target = validateOutputFile(fileName);
109124

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(",");
125+
try (BufferedWriter bw = new BufferedWriter(
126+
new OutputStreamWriter(new FileOutputStream(target), StandardCharsets.UTF_8))) {
127+
bw.write(Joiner.on(",").join(headers));
128+
bw.newLine();
129+
for (String[] record : recordsList) {
130+
if (record.length != headers.length) {
131+
continue;
132+
}
133+
StringBuilder sb = new StringBuilder();
134+
for (int i = 0; i < record.length; i++) {
135+
sb.append(!Strings.isNullOrEmpty(record[i]) ? record[i] : " ");
136+
if (i < record.length - 1) {
137+
sb.append(",");
138+
}
115139
}
140+
bw.write(sb.toString());
141+
bw.newLine();
116142
}
117-
bw.write(sb.toString());
118-
bw.newLine();
119143
}
120-
bw.flush();
121-
bw.close();
122144
}
123145
}

0 commit comments

Comments
 (0)