diff --git a/README.md b/README.md index c99b343..fdec3d5 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,33 @@ public Response generateExport() { This behavior will be available for Excel once https://github.com/Coreoz/Windmill/issues/3 is resolved. +CSV export with formula sanitization +------------------------------------ +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 `'`. + +This behavior is enabled by default. However, if you need to export actual formulas, you can disable it globally or for specific fields: + +```java +// Disable globally +Windmill + .export(data) + .asCsv(ExportCsvConfig.builder() + .disableFormulasSanitization() + .build()); + +// Disable for specific fields +Windmill + .export(data) + .withHeaderMapping( + new ExportHeaderMapping() + .add("Formula", Bean::getFormula) + .add("Value", Bean::getValue) + ) + .asCsv(ExportCsvConfig.builder() + .disableFormulasSanitizationForFields("Formula") + .build()); +``` + Excel customization for exports ------------------------------- Windmill enables full control over Excel sheets using the included `ExcelCellStyler` feature or using Apache POI. diff --git a/src/main/java/com/coreoz/windmill/exports/exporters/csv/CsvExporter.java b/src/main/java/com/coreoz/windmill/exports/exporters/csv/CsvExporter.java index cbc9ad4..b23b5f2 100644 --- a/src/main/java/com/coreoz/windmill/exports/exporters/csv/CsvExporter.java +++ b/src/main/java/com/coreoz/windmill/exports/exporters/csv/CsvExporter.java @@ -1,105 +1,129 @@ package com.coreoz.windmill.exports.exporters.csv; +import com.coreoz.windmill.exports.config.ExportMapping; +import com.coreoz.windmill.files.BomCharset; +import com.opencsv.CSVWriter; +import lombok.SneakyThrows; + import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.util.List; -import com.coreoz.windmill.files.BomCharset; -import com.coreoz.windmill.exports.config.ExportMapping; -import com.opencsv.CSVWriter; - -import lombok.SneakyThrows; - public class CsvExporter { - - private final Iterable rows; - private final ExportMapping mapping; - private final ExportCsvConfig exportConfig; - private CSVWriter csvWriter; - - public CsvExporter(Iterable rows, ExportMapping mapping, ExportCsvConfig exportConfig) { - this.rows = rows; - this.mapping = mapping; - this.exportConfig = exportConfig; - } - - /** - * Write the export file in an existing {@link OutputStream}. - * - * This {@link OutputStream} will not be closed automatically: - * it should be closed manually after this method is called. - * - * @throws IOException if anything can't be written. - */ - @SneakyThrows - public OutputStream writeTo(OutputStream outputStream) { - csvWriter = new CSVWriter( - new OutputStreamWriter(outputStream, exportConfig.getCharset().getCharset()), - exportConfig.getSeparator(), - exportConfig.getQuoteChar(), - exportConfig.getEscapeChar(), - exportConfig.getLineEnd() - ); - writeBom(outputStream); - writeRows(); - return outputStream; - } - - /** - * @throws IOException if anything can't be written. - */ - public byte[] toByteArray() { - ByteArrayOutputStream byteOutputStream = new ByteArrayOutputStream(); - writeTo(byteOutputStream); - return byteOutputStream.toByteArray(); - } - - // internals - - @SneakyThrows - private void writeBom(OutputStream outputStream) { - BomCharset encodingCharset = exportConfig.getCharset(); - if (encodingCharset != null) { + private final Iterable rows; + private final ExportMapping mapping; + private final ExportCsvConfig exportConfig; + private CSVWriter csvWriter; + + public CsvExporter(Iterable rows, ExportMapping mapping, ExportCsvConfig exportConfig) { + this.rows = rows; + this.mapping = mapping; + this.exportConfig = exportConfig; + } + + /** + * Write the export file in an existing {@link OutputStream}. + *

+ * This {@link OutputStream} will not be closed automatically: + * it should be closed manually after this method is called. + * + * @throws IOException if anything can't be written. + */ + @SneakyThrows + public OutputStream writeTo(OutputStream outputStream) { + csvWriter = new CSVWriter( + new OutputStreamWriter(outputStream, exportConfig.getCharset().getCharset()), + exportConfig.getSeparator(), + exportConfig.getQuoteChar(), + exportConfig.getEscapeChar(), + exportConfig.getLineEnd() + ); + writeBom(outputStream); + writeRows(); + return outputStream; + } + + /** + * @throws IOException if anything can't be written. + */ + public byte[] toByteArray() { + ByteArrayOutputStream byteOutputStream = new ByteArrayOutputStream(); + writeTo(byteOutputStream); + return byteOutputStream.toByteArray(); + } + + // internals + + @SneakyThrows + private void writeBom(OutputStream outputStream) { + BomCharset encodingCharset = exportConfig.getCharset(); + if (encodingCharset != null) { encodingCharset.writeBomBytes(outputStream); - } - } - - private void writeRows() { - writeHeaderRow(); - - for(T row : rows) { - writeRow(row); - } - } - - private void writeHeaderRow() { - List headerColumn = mapping.headerColumns(); - if(!headerColumn.isEmpty()) { - String[] csvRowValues = new String[headerColumn.size()]; - for (int i = 0; i < headerColumn.size(); i++) { - csvRowValues[i] = stringValue(headerColumn.get(i)); - } - csvWriter.writeNext(csvRowValues,exportConfig.isApplyQuotesToAll()); - } - } - - @SneakyThrows - private void writeRow(T row) { - String[] csvRowValues = new String[mapping.columnsCount()]; - for (int i = 0; i < mapping.columnsCount(); i++) { - csvRowValues[i] = stringValue(mapping.cellValue(i, row)); - } - csvWriter.writeNext(csvRowValues, exportConfig.isApplyQuotesToAll()); - csvWriter.flush(); - } - - private String stringValue(final Object object) { - if (object == null) { - return ""; - } - return object.toString(); - } + } + } + + private void writeRows() { + writeHeaderRow(); + + for (T row : rows) { + writeRow(row); + } + } + + private void writeHeaderRow() { + List headerColumn = mapping.headerColumns(); + if (!headerColumn.isEmpty()) { + String[] csvRowValues = new String[headerColumn.size()]; + for (int i = 0; i < headerColumn.size(); i++) { + csvRowValues[i] = sanitizeValue(headerColumn.get(i), stringValue(headerColumn.get(i))); + } + csvWriter.writeNext(csvRowValues, exportConfig.isApplyQuotesToAll()); + } + } + + @SneakyThrows + private void writeRow(T row) { + String[] csvRowValues = new String[mapping.columnsCount()]; + List headerColumns = mapping.headerColumns(); + for (int i = 0; i < mapping.columnsCount(); i++) { + String columnName = i < headerColumns.size() ? headerColumns.get(i) : null; + csvRowValues[i] = sanitizeValue(columnName, stringValue(mapping.cellValue(i, row))); + } + csvWriter.writeNext(csvRowValues, exportConfig.isApplyQuotesToAll()); + csvWriter.flush(); + } + + private String sanitizeValue(String columnName, String value) { + if ( + exportConfig.isSanitizeFormulas() + && (columnName == null || !exportConfig.getFieldNamesExcludedFromSanitization().contains(columnName)) + && isDangerousValue(value) + ) { + return "'" + value; + } + return value; + } + + private static boolean isDangerousValue(String value) { + if (value == null || value.isEmpty()) { + return false; + } + char firstChar = value.charAt(0); + return firstChar == '=' + || firstChar == '+' + || firstChar == '-' + || firstChar == '@' + || firstChar == '\t' + || firstChar == '\r'; + } + + private static String stringValue(final Object object) { + if (object == null) { + return ""; + } + return object.toString(); + } } diff --git a/src/main/java/com/coreoz/windmill/exports/exporters/csv/ExportCsvConfig.java b/src/main/java/com/coreoz/windmill/exports/exporters/csv/ExportCsvConfig.java index e1e818a..b740fc2 100644 --- a/src/main/java/com/coreoz/windmill/exports/exporters/csv/ExportCsvConfig.java +++ b/src/main/java/com/coreoz/windmill/exports/exporters/csv/ExportCsvConfig.java @@ -1,9 +1,12 @@ package com.coreoz.windmill.exports.exporters.csv; +import java.util.Collections; +import java.util.Set; + import com.coreoz.windmill.files.BomCharset; -import com.opencsv.CSVWriter; import com.opencsv.ICSVParser; +import com.opencsv.ICSVWriter; import lombok.Builder; import lombok.Value; @@ -23,7 +26,26 @@ public class ExportCsvConfig { /** The character to use for escaping quoteChar or escapeChar */ @Builder.Default private final char escapeChar = ICSVParser.DEFAULT_ESCAPE_CHARACTER; /** The line feed terminator to use */ - @Builder.Default private final String lineEnd = CSVWriter.DEFAULT_LINE_END; + @Builder.Default private final String lineEnd = ICSVWriter.DEFAULT_LINE_END; /** The boolean to use for applying or not optional wrapping quotes */ @Builder.Default private final boolean applyQuotesToAll = true; + + /** If true, values starting with =, +, -, @, \t, \r will be prefixed with ' to prevent CSV formula injection */ + @Builder.Default private final boolean sanitizeFormulas = true; + /** The field names for which formula sanitization should be disabled */ + @Builder.Default private final Set fieldNamesExcludedFromSanitization = Collections.emptySet(); + + public static class ExportCsvConfigBuilder { + public ExportCsvConfigBuilder disableFormulasSanitization() { + this.sanitizeFormulas$value = false; + this.sanitizeFormulas$set = true; + return this; + } + + public ExportCsvConfigBuilder disableFormulasSanitizationForFields(String... fieldNames) { + this.fieldNamesExcludedFromSanitization$value = Set.of(fieldNames); + this.fieldNamesExcludedFromSanitization$set = true; + return this; + } + } } diff --git a/src/test/java/com/coreoz/windmill/exports/exporters/csv/CsvFormulaInjectionTest.java b/src/test/java/com/coreoz/windmill/exports/exporters/csv/CsvFormulaInjectionTest.java new file mode 100644 index 0000000..3b3034a --- /dev/null +++ b/src/test/java/com/coreoz/windmill/exports/exporters/csv/CsvFormulaInjectionTest.java @@ -0,0 +1,92 @@ +package com.coreoz.windmill.exports.exporters.csv; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; + +import lombok.Getter; +import org.junit.Test; + +import com.coreoz.windmill.Windmill; +import com.coreoz.windmill.exports.config.ExportHeaderMapping; + +public class CsvFormulaInjectionTest { + + @Test + public void should_sanitize_formula_injection_by_default() { + String formula = "=SUM(1,2)"; + byte[] csvExport = Windmill + .export(Collections.singletonList(new RowData(formula))) + .withHeaderMapping(new ExportHeaderMapping().add("col", RowData::getValue)) + .asCsv() + .toByteArray(); + + String result = new String(csvExport, StandardCharsets.UTF_8); + // On s'attend à ce que la formule soit préfixée par ' + assertThat(result).contains("'=SUM(1,2)"); + } + + @Test + public void should_sanitize_other_dangerous_characters() { + for (String prefix : Arrays.asList("+", "-", "@", "\t", "\r")) { + String value = prefix + "danger"; + byte[] csvExport = Windmill + .export(Collections.singletonList(new RowData(value))) + .withHeaderMapping(new ExportHeaderMapping().add("col", RowData::getValue)) + .asCsv() + .toByteArray(); + + String result = new String(csvExport, StandardCharsets.UTF_8); + assertThat(result).as("Should sanitize value starting with " + prefix).contains("'" + prefix + "danger"); + } + } + + @Test + public void should_not_sanitize_formula_if_disabled_globally() { + String formula = "=SUM(1,2)"; + byte[] csvExport = Windmill + .export(Collections.singletonList(new RowData(formula))) + .withHeaderMapping(new ExportHeaderMapping().add("col", RowData::getValue)) + .asCsv( + ExportCsvConfig.builder() + .disableFormulasSanitization().build() + ) + .toByteArray(); + + String result = new String(csvExport, StandardCharsets.UTF_8); + assertThat(result) + .contains("\"=SUM(1,2)\"") + .doesNotContain("'=SUM(1,2)"); + } + + @Test + public void should_not_sanitize_formula_for_specific_fields() { + String formula = "=SUM(1,2)"; + byte[] csvExport = Windmill + .export(Collections.singletonList(new RowData(formula))) + .withHeaderMapping(new ExportHeaderMapping().add("safe", RowData::getValue).add("unsafe", RowData::getValue)) + .asCsv( + ExportCsvConfig.builder() + .disableFormulasSanitizationForFields("safe").build() + ) + .toByteArray(); + + String result = new String(csvExport, StandardCharsets.UTF_8); + assertThat(result) + // "safe" column should not be sanitized + .contains("\"=SUM(1,2)\"") + // "unsafe" column should be sanitized + .contains("\"'=SUM(1,2)\""); + } + + private static class RowData { + @Getter + private final String value; + + public RowData(String value) { + this.value = value; + } + } +}