Skip to content

Commit 3d0d42f

Browse files
author
drighetto
committed
Add method to check file/folder path and fix the javadoc formatting error #1
1 parent e03a2ad commit 3d0d42f

File tree

2 files changed

+131
-48
lines changed

2 files changed

+131
-48
lines changed

src/main/java/eu/righettod/SecurityUtils.java

Lines changed: 100 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,12 @@ private SecurityUtils() {
8080
/**
8181
* Apply a collection of validation to verify if a provided PIN code is considered weak (easy to guess) or none.<br>
8282
* This method consider that format of the PIN code is [0-9]{6,}<br>
83-
* Rule to consider a PIN code as weak:<br>
84-
* - Length is inferior to 6 positions.<br>
85-
* - Contain only the same number or only a sequence of zero.<br>
86-
* - Contain sequence of following incremental or decremental numbers.<br>
83+
* Rule to consider a PIN code as weak:
84+
* <ul>
85+
* <li>Length is inferior to 6 positions.</li>
86+
* <li>Contain only the same number or only a sequence of zero.</li>
87+
* <li>Contain sequence of following incremental or decremental numbers.</li>
88+
* </ul>
8789
*
8890
* @param pinCode PIN code to verify.
8991
* @return True only if the PIN is considered as weak.
@@ -120,10 +122,12 @@ public static boolean isWeakPINCode(String pinCode) {
120122
}
121123

122124
/**
123-
* Apply a collection of validations on a Word 97-2003 (binary format) document file provided:<br>
124-
* - Real Microsoft Word 97-2003 document file.<br>
125-
* - No VBA Macro.<br>
126-
* - No embedded objects.<br>
125+
* Apply a collection of validations on a Word 97-2003 (binary format) document file provided:
126+
* <ul>
127+
* <li>Real Microsoft Word 97-2003 document file.</li>
128+
* <li>No VBA Macro.<br></li>
129+
* <li>No embedded objects.</li>
130+
* </ul>
127131
*
128132
* @param wordFilePath Filename of the Word document file to check.
129133
* @return True only if the file pass all validations.
@@ -269,11 +273,13 @@ public boolean accept(PDAnnotation annotation) {
269273
}
270274

271275
/**
272-
* Apply a collection of validations on a PDF file provided:<br>
273-
* - Real PDF file<br>
274-
* - No attachments.<br>
275-
* - No Javascript code.<br>
276-
* - No links using action of type URI/Launch/RemoteGoTo/ImportData.<br>
276+
* Apply a collection of validations on a PDF file provided:
277+
* <ul>
278+
* <li>Real PDF file.</li>
279+
* <li>No attachments.</li>
280+
* <li>No Javascript code.</li>
281+
* <li>No links using action of type URI/Launch/RemoteGoTo/ImportData.</li>
282+
* </ul>
277283
*
278284
* @param pdfFilePath Filename of the PDF file to check.
279285
* @return True only if the file pass all validations.
@@ -377,10 +383,12 @@ public static boolean isRelativeURL(String targetUrl) {
377383
}
378384

379385
/**
380-
* Apply a collection of validations on a ZIP file provided:<br>
381-
* - Real ZIP file<br>
382-
* - Contain less than a specified level of deepness.<br>
383-
* - Do not contain Zip-Slip entry path.<br>
386+
* Apply a collection of validations on a ZIP file provided:
387+
* <ul>
388+
* <li>Real ZIP file.</li>
389+
* <li>Contain less than a specified level of deepness.</li>
390+
* <li>Do not contain Zip-Slip entry path.</li>
391+
* </ul>
384392
*
385393
* @param zipFilePath Filename of the ZIP file to check.
386394
* @param maxLevelDeepness Threshold of deepness above which a ZIP archive will be rejected.
@@ -479,11 +487,14 @@ public static String identifyMimeType(byte[] content) {
479487
}
480488

481489
/**
482-
* Apply a collection of validations on a string expected to be an public IP address:<br>
483-
* - Is a valid IP v4 or v6 address.<br>
484-
* - Is public from an Internet perspective.<br><br>
485-
* <b>Note:</b> I often see missing such validation in the value read from HTTP request headers like "X-Forwarded-For" or "Forwarded".
490+
* Apply a collection of validations on a string expected to be an public IP address:
491+
* <ul>
492+
* <li>Is a valid IP v4 or v6 address.</li>
493+
* <li>Is public from an Internet perspective.</li>
494+
* </ul>
486495
* <br>
496+
* <b>Note:</b> I often see missing such validation in the value read from HTTP request headers like "X-Forwarded-For" or "Forwarded".
497+
* <br><br>
487498
* <b>Note for IPv6:</b> I used documentation found so it is really experimental!
488499
*
489500
* @param ip String expected to be a valid IP address.
@@ -553,9 +564,12 @@ public static boolean isPublicIPAddress(String ip) {
553564
/**
554565
* Compute a SHA256 hash from an input composed of a collection of strings.<br><br>
555566
* This method take care to build the source string in a way to prevent this source string to be prone to abuse targeting the different parts composing it.<br><br>
567+
* <p>
556568
* Example of possible abuse without precautions applied during the hash calculation logic:<br>
557-
* Hash of <code>SHA256("Hello", "My", "World!!!")</code> will be equals to the hash of <code>SHA256("Hell", "oMyW", "orld!!!")</code>.<br><br>
569+
* Hash of <code>SHA256("Hello", "My", "World!!!")</code> will be equals to the hash of <code>SHA256("Hell", "oMyW", "orld!!!")</code>.<br>
570+
* </p>
558571
* This method ensure that both hash above will be different.<br><br>
572+
*
559573
* <b>Note:</b> The character <code>|</code> is used, as separator, of every parts so a part is not allowed to contains this character.
560574
*
561575
* @param parts Ordered list of strings to use to build the input string for which the hash must be computed on. No null value is accepted on object composing the collection.
@@ -644,9 +658,11 @@ public static boolean isXMLOnlyUseAllowedXSDorDTD(String xmlFilePath, final List
644658
}
645659

646660
/**
647-
* Apply a collection of validations on a EXCEL CSV file provided (file was expected to be opened in Microsoft EXCEL):<br>
648-
* - Real CSV file.<br>
649-
* - Do not contains any payload related to a CSV injections.<br><br>
661+
* Apply a collection of validations on a EXCEL CSV file provided (file was expected to be opened in Microsoft EXCEL):
662+
* <ul>
663+
* <li>Real CSV file.</li>
664+
* <li>Do not contains any payload related to a CSV injections.</li>
665+
* </ul>
650666
* Ensure that, if Apache Commons CSV does not find any record then, the file will be considered as NOT safe (prevent potential bypasses).<br><br>
651667
* <b>Note:</b> Record delimiter used is the <code>,</code> (comma) character. See the Apache Commons CSV reference provided for EXCEL.<br>
652668
*
@@ -754,11 +770,13 @@ public static Map<String, Object> ensureSerializedObjectIntegrity(ProcessingMode
754770
}
755771

756772
/**
757-
* Apply a collection of validations on a JSON string provided:<br>
758-
* - Real JSON structure.<br>
759-
* - Contain less than a specified number of deepness for nested objects or arrays.<br>
760-
* - Contain less than a specified number of items in any arrays.<br><br>
761-
*
773+
* Apply a collection of validations on a JSON string provided:
774+
* <ul>
775+
* <li>Real JSON structure.</li>
776+
* <li>Contain less than a specified number of deepness for nested objects or arrays.</li>
777+
* <li>Contain less than a specified number of items in any arrays.</li>
778+
* </ul>
779+
* <br>
762780
* <b>Note:</b> I decided to use a parsing approach using only string processing to prevent any StackOverFlow or OutOfMemory error that can be abused.<br><br>
763781
* I used the following assumption:
764782
* <ul>
@@ -863,11 +881,12 @@ public static boolean isJSONSafe(String json, int maxItemsByArraysCount, int max
863881
}
864882

865883
/**
866-
* Apply a collection of validations on a image file provided:<br>
867-
* - Real image file.<br>
868-
* - Its mime type is into the list of allowed mime types.<br>
869-
* - Its metadata fields do not contains any characters related to a malicious payloads.<br>
870-
*
884+
* Apply a collection of validations on a image file provided:
885+
* <ul>
886+
* <li>Real image file.</li>
887+
* <li>Its mime type is into the list of allowed mime types.</li>
888+
* <li>Its metadata fields do not contains any characters related to a malicious payloads.</li>
889+
* </ul>
871890
* <br>
872891
* <b>Important note:</b> This implementation is prone to bypass using the "<b>raw insertion</b>" method documented in the <a href="https://www.synacktiv.com/en/publications/persistent-php-payloads-in-pngs-how-to-inject-php-code-in-an-image-and-keep-it-there">blog post</a> from the Synacktiv team.
873892
* To handle such case, it is recommended to resize the image to remove any non image-related content, see <a href="https://github.com/righettod/document-upload-protection/blob/master/src/main/java/eu/righettod/poc/sanitizer/ImageDocumentSanitizerImpl.java#L54">here</a> for an example.<br>
@@ -982,15 +1001,17 @@ public static byte[] sanitizeFile(String inputFilePath, InputFileType inputFileT
9821001
}
9831002

9841003
/**
985-
* Apply a collection of validations on a string expected to be an email address:<br>
986-
* - Is a valid email address, from a parser perspective, following RFCs on email addresses.<br>
987-
* - Is not using "Encoded-word" format.<br>
988-
* - Is not using comment format.<br>
989-
* - Is not using "Punycode" format.<br>
990-
* - Is not using UUCP style addresses.<br>
991-
* - Is not using address literals.<br>
992-
* - Is not using source routes.<br>
993-
* - Is not using the "percent hack".<br><br>
1004+
* Apply a collection of validations on a string expected to be an email address:
1005+
* <ul>
1006+
* <li>Is a valid email address, from a parser perspective, following RFCs on email addresses.</li>
1007+
* <li>Is not using "Encoded-word" format.</li>
1008+
* <li>Is not using comment format.</li>
1009+
* <li>Is not using "Punycode" format.</li>
1010+
* <li>Is not using UUCP style addresses.</li>
1011+
* <li>Is not using address literals.</li>
1012+
* <li>Is not using source routes.</li>
1013+
* <li>Is not using the "percent hack".</li>
1014+
* </ul><br>
9941015
* This is based on the research work from <a href="https://portswigger.net/research/gareth-heyes">Gareth Heyes</a> added in references (Portswigger).<br><br>
9951016
*
9961017
* <b>Note:</b> The notion of valid, here, is to take from a secure usage of the data perspective.
@@ -1131,4 +1152,39 @@ public static String applyURLDecoding(String encodedData, int decodingRoundThres
11311152
}
11321153
return currentRoundData;
11331154
}
1155+
1156+
/**
1157+
* Apply a collection of validations on a string expected to be an system file/folder path:
1158+
* <ul>
1159+
* <li>Does not contains path traversal payload.</li>
1160+
* </ul><br>
1161+
* <p>
1162+
* <b>Note:</b> This implementation is sensitive to the current folder expression <code>./</code> and <code>.\</code> - Therefore <b>it will consider the path as unsafe</b> when it contains such expression.
1163+
* </p>
1164+
*
1165+
* @param path String expected to be a valid system file/folder path.
1166+
* @return True only if the string pass all validations.
1167+
* @see "https://portswigger.net/web-security/file-path-traversal"
1168+
* @see "https://learn.snyk.io/lesson/directory-traversal/"
1169+
* @see "https://capec.mitre.org/data/definitions/126.html"
1170+
* @see "https://owasp.org/www-community/attacks/Path_Traversal"
1171+
*/
1172+
public static boolean isPathSafe(String path) {
1173+
boolean isSafe = false;
1174+
int decodingRoundThreshold = 3;
1175+
try {
1176+
if (path != null && !path.isEmpty()) {
1177+
//URL decode the path if case of data coming from a web context
1178+
String decodedPath = applyURLDecoding(path, decodingRoundThreshold);
1179+
//Ensure that no path traversal path is present
1180+
File f = new File(decodedPath);
1181+
String canonicalPath = f.getCanonicalPath();
1182+
String absolutePath = f.getAbsolutePath();
1183+
isSafe = canonicalPath.equals(absolutePath);
1184+
}
1185+
} catch (Exception e) {
1186+
isSafe = false;
1187+
}
1188+
return isSafe;
1189+
}
11341190
}

src/test/java/eu/righettod/TestSecurityUtils.java

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -453,8 +453,8 @@ public void sanitizeFile() throws Exception {
453453

454454
@Test
455455
public void isEmailAddress() {
456-
final String templateMsgIPFalseNegative = "Email address '%s' must be detected as invalid!";
457-
final String templateMsgIPFalsePositive = "Email address '%s' must be detected as valid!";
456+
final String templateMsgFalseNegative = "Email address '%s' must be detected as invalid!";
457+
final String templateMsgFalsePositive = "Email address '%s' must be detected as valid!";
458458
//Test invalid email addresses
459459
List<String> invalidEmailAddressesList = Arrays.asList(
460460
@@ -469,7 +469,7 @@ public void isEmailAddress() {
469469
"postmaster@[IPv6:2001:0db8:85a3:0000:0000:8a2e:0370:7334]",
470470
471471
invalidEmailAddressesList.forEach(addr -> {
472-
assertFalse(SecurityUtils.isEmailAddress(addr), String.format(templateMsgIPFalseNegative, addr));
472+
assertFalse(SecurityUtils.isEmailAddress(addr), String.format(templateMsgFalseNegative, addr));
473473
});
474474
//Test valid email addresses
475475
List<String> validEmailAddressesList = Arrays.asList(
@@ -482,7 +482,7 @@ public void isEmailAddress() {
482482
"\"John..Doe\"@example.com",
483483
"\"@\"@example.com");
484484
validEmailAddressesList.forEach(addr -> {
485-
assertTrue(SecurityUtils.isEmailAddress(addr), String.format(templateMsgIPFalsePositive, addr));
485+
assertTrue(SecurityUtils.isEmailAddress(addr), String.format(templateMsgFalsePositive, addr));
486486
});
487487
}
488488

@@ -529,5 +529,32 @@ public void applyURLDecoding() {
529529
);
530530
assertTrue(thrown.getMessage().equalsIgnoreCase("Decoding round threshold of 3 reached!"));
531531
}
532+
533+
@Test
534+
public void isPathSafe() {
535+
final String templateMsgFalseNegative = "Path '%s' must be detected as invalid!";
536+
final String templateMsgFalsePositive = "Path '%s' must be detected as valid!";
537+
//Test invalid cases
538+
List<String> invalidPaths = Arrays.asList(
539+
"/home/../../../../etc/password",
540+
"%2Fhome%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2Fetc%2Fpassword", //URL encoding X1
541+
"%252Fhome%252F%252E%252E%252F%252E%252E%252F%252E%252E%252F%252E%252E%252Fetc%252Fpassword", //URL encoding X2
542+
"%25252525252Fhome%25252525252F%25252525252E%25252525252E%25252525252F%25252525252E%25252525252E%25252525252F%25252525252E%25252525252E%25252525252F%25252525252E%25252525252E%25252525252Fetc%25252525252Fpassword", //URL encoding X6
543+
"/home/..\\/..\\/..\\/..\\/etc/password"
544+
);
545+
invalidPaths.forEach(p -> {
546+
assertFalse(SecurityUtils.isPathSafe(p), String.format(templateMsgFalseNegative, p));
547+
});
548+
//Test valid cases
549+
List<String> validPaths = Arrays.asList(
550+
"/home/file",
551+
"C:\\test\\file",
552+
"test/file",
553+
"test\\file"
554+
);
555+
validPaths.forEach(p -> {
556+
assertTrue(SecurityUtils.isPathSafe(p), String.format(templateMsgFalsePositive, p));
557+
});
558+
}
532559
}
533560

0 commit comments

Comments
 (0)