feature #24508 [Serializer] Fix security issue on CsvEncoder about CSV injection (welcoMattic)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Serializer] Fix security issue on CsvEncoder about CSV injection

| Q             | A
| ------------- | ---
| Branch?       | master (4.1)
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

I read [this article](http://georgemauer.net/2017/10/07/csv-injection.html) about CSV injection and I thought it best to update the `CsvEncoder` so that it does not generate potentially malicious CSV files by default.

Commits
-------

a1b0bdbbac Fix security issue on CsvEncoder
This commit is contained in:
Fabien Potencier 2018-02-07 06:12:24 +01:00
commit dfa7bb44ac
3 changed files with 120 additions and 7 deletions

View File

@ -8,6 +8,7 @@ CHANGELOG
of objects that needs data insertion in constructor
* added an optional `default_constructor_arguments` option of context to specify a default data in
case the object is not initializable by its constructor because of data missing
* added optional `bool $escapeFormulas = false` argument to `CsvEncoder::__construct`
4.0.0
-----

View File

@ -27,18 +27,22 @@ class CsvEncoder implements EncoderInterface, DecoderInterface
const ESCAPE_CHAR_KEY = 'csv_escape_char';
const KEY_SEPARATOR_KEY = 'csv_key_separator';
const HEADERS_KEY = 'csv_headers';
const ESCAPE_FORMULAS_KEY = 'csv_escape_formulas';
private $delimiter;
private $enclosure;
private $escapeChar;
private $keySeparator;
private $escapeFormulas;
private $formulasStartCharacters = array('=', '-', '+', '@');
public function __construct(string $delimiter = ',', string $enclosure = '"', string $escapeChar = '\\', string $keySeparator = '.')
public function __construct(string $delimiter = ',', string $enclosure = '"', string $escapeChar = '\\', string $keySeparator = '.', bool $escapeFormulas = false)
{
$this->delimiter = $delimiter;
$this->enclosure = $enclosure;
$this->escapeChar = $escapeChar;
$this->keySeparator = $keySeparator;
$this->escapeFormulas = $escapeFormulas;
}
/**
@ -65,11 +69,11 @@ class CsvEncoder implements EncoderInterface, DecoderInterface
}
}
list($delimiter, $enclosure, $escapeChar, $keySeparator, $headers) = $this->getCsvOptions($context);
list($delimiter, $enclosure, $escapeChar, $keySeparator, $headers, $escapeFormulas) = $this->getCsvOptions($context);
foreach ($data as &$value) {
$flattened = array();
$this->flatten($value, $flattened, $keySeparator);
$this->flatten($value, $flattened, $keySeparator, '', $escapeFormulas);
$value = $flattened;
}
unset($value);
@ -172,13 +176,17 @@ class CsvEncoder implements EncoderInterface, DecoderInterface
/**
* Flattens an array and generates keys including the path.
*/
private function flatten(array $array, array &$result, string $keySeparator, string $parentKey = '')
private function flatten(array $array, array &$result, string $keySeparator, string $parentKey = '', bool $escapeFormulas = false)
{
foreach ($array as $key => $value) {
if (is_array($value)) {
$this->flatten($value, $result, $keySeparator, $parentKey.$key.$keySeparator);
$this->flatten($value, $result, $keySeparator, $parentKey.$key.$keySeparator, $escapeFormulas);
} else {
$result[$parentKey.$key] = $value;
if ($escapeFormulas && \in_array(substr($value, 0, 1), $this->formulasStartCharacters, true)) {
$result[$parentKey.$key] = "\t".$value;
} else {
$result[$parentKey.$key] = $value;
}
}
}
}
@ -190,12 +198,13 @@ class CsvEncoder implements EncoderInterface, DecoderInterface
$escapeChar = isset($context[self::ESCAPE_CHAR_KEY]) ? $context[self::ESCAPE_CHAR_KEY] : $this->escapeChar;
$keySeparator = isset($context[self::KEY_SEPARATOR_KEY]) ? $context[self::KEY_SEPARATOR_KEY] : $this->keySeparator;
$headers = isset($context[self::HEADERS_KEY]) ? $context[self::HEADERS_KEY] : array();
$escapeFormulas = isset($context[self::ESCAPE_FORMULAS_KEY]) ? $context[self::ESCAPE_FORMULAS_KEY] : $this->escapeFormulas;
if (!is_array($headers)) {
throw new InvalidArgumentException(sprintf('The "%s" context variable must be an array or null, given "%s".', self::HEADERS_KEY, gettype($headers)));
}
return array($delimiter, $enclosure, $escapeChar, $keySeparator, $headers);
return array($delimiter, $enclosure, $escapeChar, $keySeparator, $headers, $escapeFormulas);
}
/**

View File

@ -173,6 +173,109 @@ CSV;
$this->assertEquals($csv, $this->encoder->encode($value, 'csv', $context));
}
public function testEncodeFormulas()
{
$this->encoder = new CsvEncoder(',', '"', '\\', '.', true);
$this->assertSame(<<<'CSV'
0
" =2+3"
CSV
, $this->encoder->encode(array('=2+3'), 'csv'));
$this->assertSame(<<<'CSV'
0
" -2+3"
CSV
, $this->encoder->encode(array('-2+3'), 'csv'));
$this->assertSame(<<<'CSV'
0
" +2+3"
CSV
, $this->encoder->encode(array('+2+3'), 'csv'));
$this->assertSame(<<<'CSV'
0
" @MyDataColumn"
CSV
, $this->encoder->encode(array('@MyDataColumn'), 'csv'));
}
public function testDoNotEncodeFormulas()
{
$this->assertSame(<<<'CSV'
0
=2+3
CSV
, $this->encoder->encode(array('=2+3'), 'csv'));
$this->assertSame(<<<'CSV'
0
-2+3
CSV
, $this->encoder->encode(array('-2+3'), 'csv'));
$this->assertSame(<<<'CSV'
0
+2+3
CSV
, $this->encoder->encode(array('+2+3'), 'csv'));
$this->assertSame(<<<'CSV'
0
@MyDataColumn
CSV
, $this->encoder->encode(array('@MyDataColumn'), 'csv'));
}
public function testEncodeFormulasWithSettingsPassedInContext()
{
$this->assertSame(<<<'CSV'
0
" =2+3"
CSV
, $this->encoder->encode(array('=2+3'), 'csv', array(
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
)));
$this->assertSame(<<<'CSV'
0
" -2+3"
CSV
, $this->encoder->encode(array('-2+3'), 'csv', array(
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
)));
$this->assertSame(<<<'CSV'
0
" +2+3"
CSV
, $this->encoder->encode(array('+2+3'), 'csv', array(
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
)));
$this->assertSame(<<<'CSV'
0
" @MyDataColumn"
CSV
, $this->encoder->encode(array('@MyDataColumn'), 'csv', array(
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
)));
}
public function testSupportsDecoding()
{
$this->assertTrue($this->encoder->supportsDecoding('csv'));