Skip to content

Commit e94abe9

Browse files
amanteauxamanteaux
authored
Merge pull request #27 from Coreoz/fix/csv-formula-sanitization
Fix/csv formula sanitization
2 parents cb6af3e + 0efdbe2 commit e94abe9

4 files changed

Lines changed: 260 additions & 95 deletions

File tree

‎README.md‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,33 @@ public Response generateExport() {
177177

178178
This behavior will be available for Excel once https://github.com/Coreoz/Windmill/issues/3 is resolved.
179179

180+
CSV export with formula sanitization
181+
------------------------------------
182+
To prevent [CSV formula injection](https://owasp.org/www-community/attacks/CSV_Injection), Windmill automatically sanitizes values starting with `=`, `+`, `-`, `@`, `\t` or `\r` by prefixing them with a single quote `'`.
183+
184+
This behavior is enabled by default. However, if you need to export actual formulas, you can disable it globally or for specific fields:
185+
186+
```java
187+
// Disable globally
188+
Windmill
189+
.export(data)
190+
.asCsv(ExportCsvConfig.builder()
191+
.disableFormulasSanitization()
192+
.build());
193+
194+
// Disable for specific fields
195+
Windmill
196+
.export(data)
197+
.withHeaderMapping(
198+
new ExportHeaderMapping<Bean>()
199+
.add("Formula", Bean::getFormula)
200+
.add("Value", Bean::getValue)
201+
)
202+
.asCsv(ExportCsvConfig.builder()
203+
.disableFormulasSanitizationForFields("Formula")
204+
.build());
205+
```
206+
180207
Excel customization for exports
181208
-------------------------------
182209
Windmill enables full control over Excel sheets using the included `ExcelCellStyler` feature or using Apache POI.

‎src/main/java/com/coreoz/windmill/exports/exporters/csv/CsvExporter.java‎

Lines changed: 117 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1,105 +1,129 @@
11
package com.coreoz.windmill.exports.exporters.csv;
22

3+
import com.coreoz.windmill.exports.config.ExportMapping;
4+
import com.coreoz.windmill.files.BomCharset;
5+
import com.opencsv.CSVWriter;
6+
import lombok.SneakyThrows;
7+
38
import java.io.ByteArrayOutputStream;
49
import java.io.IOException;
510
import java.io.OutputStream;
611
import java.io.OutputStreamWriter;
712
import java.util.List;
813

9-
import com.coreoz.windmill.files.BomCharset;
10-
import com.coreoz.windmill.exports.config.ExportMapping;
11-
import com.opencsv.CSVWriter;
12-
13-
import lombok.SneakyThrows;
14-
1514
public class CsvExporter<T> {
16-
17-
private final Iterable<T> rows;
18-
private final ExportMapping<T> mapping;
19-
private final ExportCsvConfig exportConfig;
20-
private CSVWriter csvWriter;
21-
22-
public CsvExporter(Iterable<T> rows, ExportMapping<T> mapping, ExportCsvConfig exportConfig) {
23-
this.rows = rows;
24-
this.mapping = mapping;
25-
this.exportConfig = exportConfig;
26-
}
27-
28-
/**
29-
* Write the export file in an existing {@link OutputStream}.
30-
*
31-
* This {@link OutputStream} will not be closed automatically:
32-
* it should be closed manually after this method is called.
33-
*
34-
* @throws IOException if anything can't be written.
35-
*/
36-
@SneakyThrows
37-
public OutputStream writeTo(OutputStream outputStream) {
38-
csvWriter = new CSVWriter(
39-
new OutputStreamWriter(outputStream, exportConfig.getCharset().getCharset()),
40-
exportConfig.getSeparator(),
41-
exportConfig.getQuoteChar(),
42-
exportConfig.getEscapeChar(),
43-
exportConfig.getLineEnd()
44-
);
45-
writeBom(outputStream);
46-
writeRows();
47-
return outputStream;
48-
}
49-
50-
/**
51-
* @throws IOException if anything can't be written.
52-
*/
53-
public byte[] toByteArray() {
54-
ByteArrayOutputStream byteOutputStream = new ByteArrayOutputStream();
55-
writeTo(byteOutputStream);
56-
return byteOutputStream.toByteArray();
57-
}
58-
59-
// internals
60-
61-
@SneakyThrows
62-
private void writeBom(OutputStream outputStream) {
63-
BomCharset encodingCharset = exportConfig.getCharset();
64-
if (encodingCharset != null) {
15+
private final Iterable<T> rows;
16+
private final ExportMapping<T> mapping;
17+
private final ExportCsvConfig exportConfig;
18+
private CSVWriter csvWriter;
19+
20+
public CsvExporter(Iterable<T> rows, ExportMapping<T> mapping, ExportCsvConfig exportConfig) {
21+
this.rows = rows;
22+
this.mapping = mapping;
23+
this.exportConfig = exportConfig;
24+
}
25+
26+
/**
27+
* Write the export file in an existing {@link OutputStream}.
28+
* <p>
29+
* This {@link OutputStream} will not be closed automatically:
30+
* it should be closed manually after this method is called.
31+
*
32+
* @throws IOException if anything can't be written.
33+
*/
34+
@SneakyThrows
35+
public OutputStream writeTo(OutputStream outputStream) {
36+
csvWriter = new CSVWriter(
37+
new OutputStreamWriter(outputStream, exportConfig.getCharset().getCharset()),
38+
exportConfig.getSeparator(),
39+
exportConfig.getQuoteChar(),
40+
exportConfig.getEscapeChar(),
41+
exportConfig.getLineEnd()
42+
);
43+
writeBom(outputStream);
44+
writeRows();
45+
return outputStream;
46+
}
47+
48+
/**
49+
* @throws IOException if anything can't be written.
50+
*/
51+
public byte[] toByteArray() {
52+
ByteArrayOutputStream byteOutputStream = new ByteArrayOutputStream();
53+
writeTo(byteOutputStream);
54+
return byteOutputStream.toByteArray();
55+
}
56+
57+
// internals
58+
59+
@SneakyThrows
60+
private void writeBom(OutputStream outputStream) {
61+
BomCharset encodingCharset = exportConfig.getCharset();
62+
if (encodingCharset != null) {
6563
encodingCharset.writeBomBytes(outputStream);
66-
}
67-
}
68-
69-
private void writeRows() {
70-
writeHeaderRow();
71-
72-
for(T row : rows) {
73-
writeRow(row);
74-
}
75-
}
76-
77-
private void writeHeaderRow() {
78-
List<String> headerColumn = mapping.headerColumns();
79-
if(!headerColumn.isEmpty()) {
80-
String[] csvRowValues = new String[headerColumn.size()];
81-
for (int i = 0; i < headerColumn.size(); i++) {
82-
csvRowValues[i] = stringValue(headerColumn.get(i));
83-
}
84-
csvWriter.writeNext(csvRowValues,exportConfig.isApplyQuotesToAll());
85-
}
86-
}
87-
88-
@SneakyThrows
89-
private void writeRow(T row) {
90-
String[] csvRowValues = new String[mapping.columnsCount()];
91-
for (int i = 0; i < mapping.columnsCount(); i++) {
92-
csvRowValues[i] = stringValue(mapping.cellValue(i, row));
93-
}
94-
csvWriter.writeNext(csvRowValues, exportConfig.isApplyQuotesToAll());
95-
csvWriter.flush();
96-
}
97-
98-
private String stringValue(final Object object) {
99-
if (object == null) {
100-
return "";
101-
}
102-
return object.toString();
103-
}
64+
}
65+
}
66+
67+
private void writeRows() {
68+
writeHeaderRow();
69+
70+
for (T row : rows) {
71+
writeRow(row);
72+
}
73+
}
74+
75+
private void writeHeaderRow() {
76+
List<String> headerColumn = mapping.headerColumns();
77+
if (!headerColumn.isEmpty()) {
78+
String[] csvRowValues = new String[headerColumn.size()];
79+
for (int i = 0; i < headerColumn.size(); i++) {
80+
csvRowValues[i] = sanitizeValue(headerColumn.get(i), stringValue(headerColumn.get(i)));
81+
}
82+
csvWriter.writeNext(csvRowValues, exportConfig.isApplyQuotesToAll());
83+
}
84+
}
85+
86+
@SneakyThrows
87+
private void writeRow(T row) {
88+
String[] csvRowValues = new String[mapping.columnsCount()];
89+
List<String> headerColumns = mapping.headerColumns();
90+
for (int i = 0; i < mapping.columnsCount(); i++) {
91+
String columnName = i < headerColumns.size() ? headerColumns.get(i) : null;
92+
csvRowValues[i] = sanitizeValue(columnName, stringValue(mapping.cellValue(i, row)));
93+
}
94+
csvWriter.writeNext(csvRowValues, exportConfig.isApplyQuotesToAll());
95+
csvWriter.flush();
96+
}
97+
98+
private String sanitizeValue(String columnName, String value) {
99+
if (
100+
exportConfig.isSanitizeFormulas()
101+
&& (columnName == null || !exportConfig.getFieldNamesExcludedFromSanitization().contains(columnName))
102+
&& isDangerousValue(value)
103+
) {
104+
return "'" + value;
105+
}
106+
return value;
107+
}
108+
109+
private static boolean isDangerousValue(String value) {
110+
if (value == null || value.isEmpty()) {
111+
return false;
112+
}
113+
char firstChar = value.charAt(0);
114+
return firstChar == '='
115+
|| firstChar == '+'
116+
|| firstChar == '-'
117+
|| firstChar == '@'
118+
|| firstChar == '\t'
119+
|| firstChar == '\r';
120+
}
121+
122+
private static String stringValue(final Object object) {
123+
if (object == null) {
124+
return "";
125+
}
126+
return object.toString();
127+
}
104128

105129
}

‎src/main/java/com/coreoz/windmill/exports/exporters/csv/ExportCsvConfig.java‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package com.coreoz.windmill.exports.exporters.csv;
22

3+
import java.util.Collections;
4+
import java.util.Set;
5+
36
import com.coreoz.windmill.files.BomCharset;
4-
import com.opencsv.CSVWriter;
57
import com.opencsv.ICSVParser;
68

9+
import com.opencsv.ICSVWriter;
710
import lombok.Builder;
811
import lombok.Value;
912

@@ -23,7 +26,26 @@ public class ExportCsvConfig {
2326
/** The character to use for escaping quoteChar or escapeChar */
2427
@Builder.Default private final char escapeChar = ICSVParser.DEFAULT_ESCAPE_CHARACTER;
2528
/** The line feed terminator to use */
26-
@Builder.Default private final String lineEnd = CSVWriter.DEFAULT_LINE_END;
29+
@Builder.Default private final String lineEnd = ICSVWriter.DEFAULT_LINE_END;
2730
/** The boolean to use for applying or not optional wrapping quotes */
2831
@Builder.Default private final boolean applyQuotesToAll = true;
32+
33+
/** If true, values starting with =, +, -, @, \t, \r will be prefixed with ' to prevent CSV formula injection */
34+
@Builder.Default private final boolean sanitizeFormulas = true;
35+
/** The field names for which formula sanitization should be disabled */
36+
@Builder.Default private final Set<String> fieldNamesExcludedFromSanitization = Collections.emptySet();
37+
38+
public static class ExportCsvConfigBuilder {
39+
public ExportCsvConfigBuilder disableFormulasSanitization() {
40+
this.sanitizeFormulas$value = false;
41+
this.sanitizeFormulas$set = true;
42+
return this;
43+
}
44+
45+
public ExportCsvConfigBuilder disableFormulasSanitizationForFields(String... fieldNames) {
46+
this.fieldNamesExcludedFromSanitization$value = Set.of(fieldNames);
47+
this.fieldNamesExcludedFromSanitization$set = true;
48+
return this;
49+
}
50+
}
2951
}

‎src/test/java/com/coreoz/windmill/exports/exporters/csv/CsvFormulaInjectionTest.java‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package com.coreoz.windmill.exports.exporters.csv;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.nio.charset.StandardCharsets;
6+
import java.util.Arrays;
7+
import java.util.Collections;
8+
9+
import lombok.Getter;
10+
import org.junit.Test;
11+
12+
import com.coreoz.windmill.Windmill;
13+
import com.coreoz.windmill.exports.config.ExportHeaderMapping;
14+
15+
public class CsvFormulaInjectionTest {
16+
17+
@Test
18+
public void should_sanitize_formula_injection_by_default() {
19+
String formula = "=SUM(1,2)";
20+
byte[] csvExport = Windmill
21+
.export(Collections.singletonList(new RowData(formula)))
22+
.withHeaderMapping(new ExportHeaderMapping<RowData>().add("col", RowData::getValue))
23+
.asCsv()
24+
.toByteArray();
25+
26+
String result = new String(csvExport, StandardCharsets.UTF_8);
27+
// On s'attend à ce que la formule soit préfixée par '
28+
assertThat(result).contains("'=SUM(1,2)");
29+
}
30+
31+
@Test
32+
public void should_sanitize_other_dangerous_characters() {
33+
for (String prefix : Arrays.asList("+", "-", "@", "\t", "\r")) {
34+
String value = prefix + "danger";
35+
byte[] csvExport = Windmill
36+
.export(Collections.singletonList(new RowData(value)))
37+
.withHeaderMapping(new ExportHeaderMapping<RowData>().add("col", RowData::getValue))
38+
.asCsv()
39+
.toByteArray();
40+
41+
String result = new String(csvExport, StandardCharsets.UTF_8);
42+
assertThat(result).as("Should sanitize value starting with " + prefix).contains("'" + prefix + "danger");
43+
}
44+
}
45+
46+
@Test
47+
public void should_not_sanitize_formula_if_disabled_globally() {
48+
String formula = "=SUM(1,2)";
49+
byte[] csvExport = Windmill
50+
.export(Collections.singletonList(new RowData(formula)))
51+
.withHeaderMapping(new ExportHeaderMapping<RowData>().add("col", RowData::getValue))
52+
.asCsv(
53+
ExportCsvConfig.builder()
54+
.disableFormulasSanitization().build()
55+
)
56+
.toByteArray();
57+
58+
String result = new String(csvExport, StandardCharsets.UTF_8);
59+
assertThat(result)
60+
.contains("\"=SUM(1,2)\"")
61+
.doesNotContain("'=SUM(1,2)");
62+
}
63+
64+
@Test
65+
public void should_not_sanitize_formula_for_specific_fields() {
66+
String formula = "=SUM(1,2)";
67+
byte[] csvExport = Windmill
68+
.export(Collections.singletonList(new RowData(formula)))
69+
.withHeaderMapping(new ExportHeaderMapping<RowData>().add("safe", RowData::getValue).add("unsafe", RowData::getValue))
70+
.asCsv(
71+
ExportCsvConfig.builder()
72+
.disableFormulasSanitizationForFields("safe").build()
73+
)
74+
.toByteArray();
75+
76+
String result = new String(csvExport, StandardCharsets.UTF_8);
77+
assertThat(result)
78+
// "safe" column should not be sanitized
79+
.contains("\"=SUM(1,2)\"")
80+
// "unsafe" column should be sanitized
81+
.contains("\"'=SUM(1,2)\"");
82+
}
83+
84+
private static class RowData {
85+
@Getter
86+
private final String value;
87+
88+
public RowData(String value) {
89+
this.value = value;
90+
}
91+
}
92+
}

0 commit comments

Comments
 (0)