Fix/csv formula sanitization

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<Bean>()
+      .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.

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<T> {
-
-	private final Iterable<T> rows;
-	private final ExportMapping<T> mapping;
-	private final ExportCsvConfig exportConfig;
-	private CSVWriter csvWriter;
-
-	public CsvExporter(Iterable<T> rows, ExportMapping<T> 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<T> rows;
+    private final ExportMapping<T> mapping;
+    private final ExportCsvConfig exportConfig;
+    private CSVWriter csvWriter;
+
+    public CsvExporter(Iterable<T> rows, ExportMapping<T> mapping, ExportCsvConfig exportConfig) {
+        this.rows = rows;
+        this.mapping = mapping;
+        this.exportConfig = exportConfig;
+    }
+
+    /**
+     * Write the export file in an existing {@link OutputStream}.
+     * <p>
+     * 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<String> 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<String> 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<String> 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();
+    }
 
 }

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<String> 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;
+		}
+	}
 }

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<RowData>().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<RowData>().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<RowData>().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<RowData>().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;
+        }
+    }
+}