Skip to content

Commit afe78a0

Browse files
krosenvoldkrosenvold
committed
IO-484 Fix with testcase
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/io/trunk@1694464 13f79535-47bb-0310-9956-ffa450edef68
1 parent 35ea189 commit afe78a0

3 files changed

Lines changed: 77 additions & 17 deletions

File tree

‎src/changes/changes.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ The <action> type attribute can be add,update,fix,remove.
4747
<body>
4848
<!-- The release date is the date RC is cut -->
4949
<release version="2.5" date="2015-??-??" description="New features and bug fixes.">
50+
<action issue="IO-484" dev="krosenvold" type="fix" due-to="Philippe Arteau">
51+
FilenameUtils should handle embedded null bytes
52+
</action>
5053
<action issue="IO-481" dev="krosenvold" type="fix">
5154
Changed/Corrected algorithm for waitFor
5255
</action>

‎src/main/java/org/apache/commons/io/FilenameUtils.java‎

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ private static boolean isSeparator(final char ch) {
190190
* (Note the file separator returned will be correct for Windows/Unix)
191191
*
192192
* @param filename the filename to normalize, null returns null
193-
* @return the normalized filename, or null if invalid
193+
* @return the normalized filename, or null if invalid. Null bytes inside string will be removed
194194
*/
195195
public static String normalize(final String filename) {
196196
return doNormalize(filename, SYSTEM_SEPARATOR, true);
@@ -236,7 +236,7 @@ public static String normalize(final String filename) {
236236
* @param filename the filename to normalize, null returns null
237237
* @param unixSeparator {@code true} if a unix separator should
238238
* be used or {@code false} if a windows separator should be used.
239-
* @return the normalized filename, or null if invalid
239+
* @return the normalized filename, or null if invalid. Null bytes inside string will be removed
240240
* @since 2.0
241241
*/
242242
public static String normalize(final String filename, final boolean unixSeparator) {
@@ -284,7 +284,7 @@ public static String normalize(final String filename, final boolean unixSeparato
284284
* (Note the file separator returned will be correct for Windows/Unix)
285285
*
286286
* @param filename the filename to normalize, null returns null
287-
* @return the normalized filename, or null if invalid
287+
* @return the normalized filename, or null if invalid. Null bytes inside string will be removed
288288
*/
289289
public static String normalizeNoEndSeparator(final String filename) {
290290
return doNormalize(filename, SYSTEM_SEPARATOR, false);
@@ -330,7 +330,7 @@ public static String normalizeNoEndSeparator(final String filename) {
330330
* @param filename the filename to normalize, null returns null
331331
* @param unixSeparator {@code true} if a unix separator should
332332
* be used or {@code false} if a windows separtor should be used.
333-
* @return the normalized filename, or null if invalid
333+
* @return the normalized filename, or null if invalid. Null bytes inside string will be removed
334334
* @since 2.0
335335
*/
336336
public static String normalizeNoEndSeparator(final String filename, final boolean unixSeparator) {
@@ -344,23 +344,26 @@ public static String normalizeNoEndSeparator(final String filename, final boolea
344344
* @param filename the filename
345345
* @param separator The separator character to use
346346
* @param keepSeparator true to keep the final separator
347-
* @return the normalized filename
347+
* @return the normalized filename. Null bytes inside string will be removed.
348348
*/
349349
private static String doNormalize(final String filename, final char separator, final boolean keepSeparator) {
350350
if (filename == null) {
351351
return null;
352352
}
353-
int size = filename.length();
353+
354+
String cleanFileName = filterNullBytes(filename);
355+
356+
int size = cleanFileName.length();
354357
if (size == 0) {
355-
return filename;
358+
return cleanFileName;
356359
}
357-
final int prefix = getPrefixLength(filename);
360+
final int prefix = getPrefixLength(cleanFileName);
358361
if (prefix < 0) {
359362
return null;
360363
}
361364

362365
final char[] array = new char[size + 2]; // +1 for possible extra slash, +2 for arraycopy
363-
filename.getChars(0, filename.length(), array, 0);
366+
cleanFileName.getChars(0, cleanFileName.length(), array, 0);
364367

365368
// fix separators throughout
366369
final char otherSeparator = separator == SYSTEM_SEPARATOR ? OTHER_SEPARATOR : SYSTEM_SEPARATOR;
@@ -478,7 +481,7 @@ private static String doNormalize(final String filename, final char separator, f
478481
*
479482
* @param basePath the base path to attach to, always treated as a path
480483
* @param fullFilenameToAdd the filename (or path) to attach to the base
481-
* @return the concatenated path, or null if invalid
484+
* @return the concatenated path, or null if invalid. Null bytes inside string will be removed
482485
*/
483486
public static String concat(final String basePath, final String fullFilenameToAdd) {
484487
final int prefix = getPrefixLength(fullFilenameToAdd);
@@ -953,14 +956,44 @@ private static String doGetFullPath(final String filename, final boolean include
953956
* The output will be the same irrespective of the machine that the code is running on.
954957
*
955958
* @param filename the filename to query, null returns null
956-
* @return the name of the file without the path, or an empty string if none exists
959+
* @return the name of the file without the path, or an empty string if none exists.
960+
* Null bytes inside string will be removed
957961
*/
958962
public static String getName(final String filename) {
959963
if (filename == null) {
960964
return null;
961965
}
962-
final int index = indexOfLastSeparator(filename);
963-
return filename.substring(index + 1);
966+
String cleanFileName = filterNullBytes(filename);
967+
final int index = indexOfLastSeparator(cleanFileName);
968+
return cleanFileName.substring(index + 1);
969+
}
970+
971+
/**
972+
* Check the input for null bytes, a sign of unsanitized data being passed to to file level functions.
973+
*
974+
* This may be used for poison byte attacks.
975+
* @param path the path to check
976+
*/
977+
private static void failIfNullBytePresent(String path) {
978+
int len = path.length();
979+
for (int i = 0; i < len; i++) {
980+
if (path.charAt(i) == 0) {
981+
throw new IllegalArgumentException("Null byte present in file/path name. There are no " +
982+
"known legitimate use cases for such data, but several injection attacks may use it");
983+
}
984+
}
985+
}
986+
987+
/**
988+
* Filters the supplied path for null byte characters. Can be used for normalizations to avoid poison byte attacks.
989+
*
990+
* This mimicks behaviour of 1.7u40+. Once minimum java requirement is above this version, this code can be removed.
991+
*
992+
* @param path the path
993+
* @return the supplied string without any embedded null characters
994+
*/
995+
private static String filterNullBytes(String path) {
996+
return path.contains("\u0000") ? path.replace("\u0000", "") : path;
964997
}
965998

966999
/**
@@ -978,7 +1011,8 @@ public static String getName(final String filename) {
9781011
* The output will be the same irrespective of the machine that the code is running on.
9791012
*
9801013
* @param filename the filename to query, null returns null
981-
* @return the name of the file without the path, or an empty string if none exists
1014+
* @return the name of the file without the path, or an empty string if none exists. Null bytes inside string
1015+
* will be removed
9821016
*/
9831017
public static String getBaseName(final String filename) {
9841018
return removeExtension(getName(filename));
@@ -1036,11 +1070,13 @@ public static String removeExtension(final String filename) {
10361070
if (filename == null) {
10371071
return null;
10381072
}
1039-
final int index = indexOfExtension(filename);
1073+
String cleanFileName = filterNullBytes(filename);
1074+
1075+
final int index = indexOfExtension(cleanFileName);
10401076
if (index == NOT_FOUND) {
1041-
return filename;
1077+
return cleanFileName;
10421078
} else {
1043-
return filename.substring(0, index);
1079+
return cleanFileName.substring(0, index);
10441080
}
10451081
}
10461082

@@ -1151,11 +1187,14 @@ public static boolean equals(
11511187
* @param filename the filename to query, null returns false
11521188
* @param extension the extension to check for, null or empty checks for no extension
11531189
* @return true if the filename has the specified extension
1190+
* @throws java.lang.IllegalArgumentException if the supplied filename contains null bytes
11541191
*/
11551192
public static boolean isExtension(final String filename, final String extension) {
11561193
if (filename == null) {
11571194
return false;
11581195
}
1196+
failIfNullBytePresent(filename);
1197+
11591198
if (extension == null || extension.isEmpty()) {
11601199
return indexOfExtension(filename) == NOT_FOUND;
11611200
}
@@ -1173,11 +1212,14 @@ public static boolean isExtension(final String filename, final String extension)
11731212
* @param filename the filename to query, null returns false
11741213
* @param extensions the extensions to check for, null checks for no extension
11751214
* @return true if the filename is one of the extensions
1215+
* @throws java.lang.IllegalArgumentException if the supplied filename contains null bytes
11761216
*/
11771217
public static boolean isExtension(final String filename, final String[] extensions) {
11781218
if (filename == null) {
11791219
return false;
11801220
}
1221+
failIfNullBytePresent(filename);
1222+
11811223
if (extensions == null || extensions.length == 0) {
11821224
return indexOfExtension(filename) == NOT_FOUND;
11831225
}
@@ -1200,11 +1242,14 @@ public static boolean isExtension(final String filename, final String[] extensio
12001242
* @param filename the filename to query, null returns false
12011243
* @param extensions the extensions to check for, null checks for no extension
12021244
* @return true if the filename is one of the extensions
1245+
* @throws java.lang.IllegalArgumentException if the supplied filename contains null bytes
12031246
*/
12041247
public static boolean isExtension(final String filename, final Collection<String> extensions) {
12051248
if (filename == null) {
12061249
return false;
12071250
}
1251+
failIfNullBytePresent(filename);
1252+
12081253
if (extensions == null || extensions.isEmpty()) {
12091254
return indexOfExtension(filename) == NOT_FOUND;
12101255
}

‎src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ public void testNormalize() throws Exception {
216216
assertEquals(null, FilenameUtils.normalize("//server/../a"));
217217
assertEquals(null, FilenameUtils.normalize("//server/.."));
218218
assertEquals(SEP + SEP + "server" + SEP + "", FilenameUtils.normalize("//server/"));
219+
assertEquals("a" + SEP + "b" + SEP + "c.txt", FilenameUtils.normalize("a\\b/c\u0000.txt"));
220+
assertEquals("a" + SEP + "b" + SEP + "c.txt", FilenameUtils.normalize("\u0000a\\b/c.txt"));
219221
}
220222

221223
public void testNormalizeUnixWin() throws Exception {
@@ -730,6 +732,7 @@ public void testGetName() {
730732
assertEquals("c", FilenameUtils.getName("a/b/c"));
731733
assertEquals("", FilenameUtils.getName("a/b/c/"));
732734
assertEquals("c", FilenameUtils.getName("a\\b\\c"));
735+
assertEquals("c", FilenameUtils.getName("a\\b\\\u0000c"));
733736
}
734737

735738
public void testGetBaseName() {
@@ -740,6 +743,7 @@ public void testGetBaseName() {
740743
assertEquals("", FilenameUtils.getBaseName("a/b/c/"));
741744
assertEquals("c", FilenameUtils.getBaseName("a\\b\\c"));
742745
assertEquals("file.txt", FilenameUtils.getBaseName("file.txt.bak"));
746+
assertEquals("file.txt", FilenameUtils.getBaseName("fil\u0000e.txt.bak"));
743747
}
744748

745749
public void testGetExtension() {
@@ -882,6 +886,14 @@ public void testIsExtension() {
882886
assertFalse(FilenameUtils.isExtension("a.b\\file.txt", "TXT"));
883887
}
884888

889+
public void testIsExtension_injection() {
890+
try {
891+
FilenameUtils.isExtension("a.b\\fi\u0000le.txt", "TXT");
892+
fail("Should throw IAE");
893+
} catch (IllegalArgumentException ignore) {
894+
}
895+
}
896+
885897
public void testIsExtensionArray() {
886898
assertFalse(FilenameUtils.isExtension(null, (String[]) null));
887899
assertFalse(FilenameUtils.isExtension("file.txt", (String[]) null));

0 commit comments

Comments
 (0)