Skip to content

Commit fe6bae7

Browse files
author
drighetto
committed
Add method to validate a Excel CSV file - Implements #1
1 parent 75f5fd4 commit fe6bae7

13 files changed

+95
-6
lines changed

.idea/inspectionProfiles/Project_Default.xml

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
<version>1.0.0-SNAPSHOT</version>
1010

1111
<dependencies>
12+
<dependency>
13+
<groupId>org.apache.commons</groupId>
14+
<artifactId>commons-csv</artifactId>
15+
<version>1.11.0</version>
16+
</dependency>
1217
<dependency>
1318
<groupId>commons-validator</groupId>
1419
<artifactId>commons-validator</artifactId>

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

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package eu.righettod;
22

3+
import org.apache.commons.csv.CSVFormat;
4+
import org.apache.commons.csv.CSVRecord;
35
import org.apache.commons.validator.routines.InetAddressValidator;
46
import org.apache.pdfbox.Loader;
57
import org.apache.pdfbox.pdmodel.PDDocument;
@@ -29,10 +31,7 @@
2931
import javax.xml.parsers.DocumentBuilder;
3032
import javax.xml.parsers.DocumentBuilderFactory;
3133
import javax.xml.parsers.ParserConfigurationException;
32-
import java.io.ByteArrayInputStream;
33-
import java.io.File;
34-
import java.io.IOException;
35-
import java.io.StringReader;
34+
import java.io.*;
3635
import java.net.Inet6Address;
3736
import java.net.InetAddress;
3837
import java.net.MalformedURLException;
@@ -41,6 +40,7 @@
4140
import java.nio.file.Files;
4241
import java.security.MessageDigest;
4342
import java.util.*;
43+
import java.util.concurrent.atomic.AtomicInteger;
4444
import java.util.regex.Pattern;
4545
import java.util.zip.ZipEntry;
4646
import java.util.zip.ZipFile;
@@ -625,4 +625,48 @@ public static boolean isXMLOnlyUseAllowedXSDorDTD(String xmlFilePath, final List
625625

626626
return isSafe;
627627
}
628+
629+
/**
630+
* Apply a collection of validations on a EXCEL CSV file provided (file was expected to be opened in Microsoft EXCEL):<br>
631+
* - Real CSV file.<br>
632+
* - Do not contains any payload related to a CSV injections.<br><br>
633+
* 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>
634+
* <b>Note:</b> Record delimiter used is the <code>,</code> (comma) character. See the Apache Commons CSV reference provided for EXCEL.<br>
635+
*
636+
* @param csvFilePath Filename of the CSV file to check.
637+
* @return True only if the file pass all validations.
638+
* @see "https://commons.apache.org/proper/commons-csv/"
639+
* @see "https://commons.apache.org/proper/commons-csv/apidocs/org/apache/commons/csv/CSVFormat.html#EXCEL"
640+
* @see "https://www.we45.com/post/your-excel-sheets-are-not-safe-heres-how-to-beat-csv-injection"
641+
* @see "https://www.whiteoaksecurity.com/blog/2020-4-23-csv-injection-whats-the-risk/"
642+
* @see "https://book.hacktricks.xyz/pentesting-web/formula-csv-doc-latex-ghostscript-injection"
643+
* @see "https://owasp.org/www-community/attacks/CSV_Injection"
644+
* @see "https://payatu.com/blog/csv-injection-basic-to-exploit/"
645+
* @see "https://cwe.mitre.org/data/definitions/1236.html"
646+
*/
647+
public static boolean isExcelCSVSafe(String csvFilePath) {
648+
boolean isSafe;
649+
final AtomicInteger recordCount = new AtomicInteger();
650+
final List<Character> payloadDetectionCharacters = List.of('=', '+', '@', '-', '\r', '\t');
651+
652+
try {
653+
final List<String> payloadsIdentified = new ArrayList<>();
654+
try (Reader in = new FileReader(csvFilePath)) {
655+
Iterable<CSVRecord> records = CSVFormat.EXCEL.parse(in);
656+
records.forEach(record -> {
657+
record.forEach(recordValue -> {
658+
if (recordValue != null && !recordValue.trim().isEmpty() && payloadDetectionCharacters.contains(recordValue.trim().charAt(0))) {
659+
payloadsIdentified.add(recordValue);
660+
}
661+
recordCount.getAndIncrement();
662+
});
663+
});
664+
}
665+
isSafe = (payloadsIdentified.isEmpty() && recordCount.get() > 0);
666+
} catch (Exception e) {
667+
isSafe = false;
668+
}
669+
670+
return isSafe;
671+
}
628672
}

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package eu.righettod;
22

33

4+
import org.apache.commons.lang3.StringUtils;
45
import org.apache.pdfbox.Loader;
56
import org.apache.pdfbox.pdmodel.PDDocument;
67
import org.junit.jupiter.api.Test;
@@ -251,7 +252,7 @@ public void computeHashNoProneToAbuseOnParts() throws Exception {
251252
}
252253

253254
@Test
254-
public void isXMLOnlyUseAllowedXSDorDTD() throws Exception {
255+
public void isXMLOnlyUseAllowedXSDorDTD() {
255256
final String msgErrorFalseNegative = "Reference to an invalid DTD/XSD must be detected!";
256257
final String msgErrorFalsePositive = "Reference to an invalid DTD/XSD must NOT be detected!";
257258
//Non allowed DTD
@@ -275,5 +276,30 @@ public void isXMLOnlyUseAllowedXSDorDTD() throws Exception {
275276
isSafe = SecurityUtils.isXMLOnlyUseAllowedXSDorDTD(testFile, List.of("https://company.com/test.xsd"));
276277
assertTrue(isSafe, msgErrorFalsePositive);
277278
}
279+
280+
@Test
281+
public void isExcelCSVSafe() throws IOException {
282+
String testFile;
283+
boolean isSafe;
284+
String caseIdentifier;
285+
286+
//Test unsafe CSV cases
287+
int unsafeTestCaseCount = 5;
288+
for (int caseId = 0; caseId < unsafeTestCaseCount; caseId++) {
289+
caseIdentifier = StringUtils.leftPad(Integer.toString(caseId), 2, "0");
290+
testFile = getTestFilePath(String.format("test-excel-csv-unsafe%s.csv", caseIdentifier));
291+
isSafe = SecurityUtils.isExcelCSVSafe(testFile);
292+
assertFalse(isSafe, String.format(TEMPLATE_MESSAGE_FALSE_NEGATIVE_FOR_FILE, StringUtils.leftPad(Integer.toString(caseId), 2, "0")));
293+
}
294+
//Test safe CSV cases
295+
int safeTestCaseCount = 4;
296+
for (int caseId = 0; caseId < safeTestCaseCount; caseId++) {
297+
caseIdentifier = StringUtils.leftPad(Integer.toString(caseId), 2, "0");
298+
testFile = getTestFilePath(String.format("test-excel-csv-safe%s.csv", caseIdentifier));
299+
isSafe = SecurityUtils.isExcelCSVSafe(testFile);
300+
assertTrue(isSafe, String.format(TEMPLATE_MESSAGE_FALSE_POSITIVE_FOR_FILE, testFile));
301+
}
302+
303+
}
278304
}
279305

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
"FIELD1","FIELD2"
2+
"Dom",70
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"Dom",70
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Dom,70
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
FIELD1,FIELD2
2+
Dom,70
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
"FIELD1","FIELD2"
2+
"Dom","=HYPERLINK(""https://evil.com"",""Click here"")"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"Dom","=HYPERLINK(""https://evil.com"",""Click here"")"
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
"FIELD1","FIELD2"
2+
"Dom","@SUM(1+9)*cmd|' /C calc'!A0"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"Dom","@SUM(1+9)*cmd|' /C calc'!A0"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
,=cmd|\' /C calc\'!A0'

0 commit comments

Comments
 (0)