Skip to content

Commit dfa7bb4

Browse files
fabpotfabpot
committed
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 ------- a1b0bdb Fix security issue on CsvEncoder
2 parents 389676c + a1b0bdb commit dfa7bb4

File tree

3 files changed

+120
-7
lines changed

3 files changed

+120
-7
lines changed

‎src/Symfony/Component/Serializer/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
of objects that needs data insertion in constructor
99
* added an optional `default_constructor_arguments` option of context to specify a default data in
1010
case the object is not initializable by its constructor because of data missing
11+
* added optional `bool $escapeFormulas = false` argument to `CsvEncoder::__construct`
1112

1213
4.0.0
1314
-----

‎src/Symfony/Component/Serializer/Encoder/CsvEncoder.php

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,22 @@ class CsvEncoder implements EncoderInterface, DecoderInterface
2727
const ESCAPE_CHAR_KEY = 'csv_escape_char';
2828
const KEY_SEPARATOR_KEY = 'csv_key_separator';
2929
const HEADERS_KEY = 'csv_headers';
30+
const ESCAPE_FORMULAS_KEY = 'csv_escape_formulas';
3031

3132
private $delimiter;
3233
private $enclosure;
3334
private $escapeChar;
3435
private $keySeparator;
36+
private $escapeFormulas;
37+
private $formulasStartCharacters = array('=', '-', '+', '@');
3538

36-
public function __construct(string $delimiter = ',', string $enclosure = '"', string $escapeChar = '\\', string $keySeparator = '.')
39+
public function __construct(string $delimiter = ',', string $enclosure = '"', string $escapeChar = '\\', string $keySeparator = '.', bool $escapeFormulas = false)
3740
{
3841
$this->delimiter = $delimiter;
3942
$this->enclosure = $enclosure;
4043
$this->escapeChar = $escapeChar;
4144
$this->keySeparator = $keySeparator;
45+
$this->escapeFormulas = $escapeFormulas;
4246
}
4347

4448
/**
@@ -65,11 +69,11 @@ public function encode($data, $format, array $context = array())
6569
}
6670
}
6771

68-
list($delimiter, $enclosure, $escapeChar, $keySeparator, $headers) = $this->getCsvOptions($context);
72+
list($delimiter, $enclosure, $escapeChar, $keySeparator, $headers, $escapeFormulas) = $this->getCsvOptions($context);
6973

7074
foreach ($data as &$value) {
7175
$flattened = array();
72-
$this->flatten($value, $flattened, $keySeparator);
76+
$this->flatten($value, $flattened, $keySeparator, '', $escapeFormulas);
7377
$value = $flattened;
7478
}
7579
unset($value);
@@ -172,13 +176,17 @@ public function supportsDecoding($format)
172176
/**
173177
* Flattens an array and generates keys including the path.
174178
*/
175-
private function flatten(array $array, array &$result, string $keySeparator, string $parentKey = '')
179+
private function flatten(array $array, array &$result, string $keySeparator, string $parentKey = '', bool $escapeFormulas = false)
176180
{
177181
foreach ($array as $key => $value) {
178182
if (is_array($value)) {
179-
$this->flatten($value, $result, $keySeparator, $parentKey.$key.$keySeparator);
183+
$this->flatten($value, $result, $keySeparator, $parentKey.$key.$keySeparator, $escapeFormulas);
180184
} else {
181-
$result[$parentKey.$key] = $value;
185+
if ($escapeFormulas && \in_array(substr($value, 0, 1), $this->formulasStartCharacters, true)) {
186+
$result[$parentKey.$key] = "\t".$value;
187+
} else {
188+
$result[$parentKey.$key] = $value;
189+
}
182190
}
183191
}
184192
}
@@ -190,12 +198,13 @@ private function getCsvOptions(array $context)
190198
$escapeChar = isset($context[self::ESCAPE_CHAR_KEY]) ? $context[self::ESCAPE_CHAR_KEY] : $this->escapeChar;
191199
$keySeparator = isset($context[self::KEY_SEPARATOR_KEY]) ? $context[self::KEY_SEPARATOR_KEY] : $this->keySeparator;
192200
$headers = isset($context[self::HEADERS_KEY]) ? $context[self::HEADERS_KEY] : array();
201+
$escapeFormulas = isset($context[self::ESCAPE_FORMULAS_KEY]) ? $context[self::ESCAPE_FORMULAS_KEY] : $this->escapeFormulas;
193202

194203
if (!is_array($headers)) {
195204
throw new InvalidArgumentException(sprintf('The "%s" context variable must be an array or null, given "%s".', self::HEADERS_KEY, gettype($headers)));
196205
}
197206

198-
return array($delimiter, $enclosure, $escapeChar, $keySeparator, $headers);
207+
return array($delimiter, $enclosure, $escapeChar, $keySeparator, $headers, $escapeFormulas);
199208
}
200209

201210
/**

‎src/Symfony/Component/Serializer/Tests/Encoder/CsvEncoderTest.php

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,109 @@ public function testEncodeCustomHeaders()
173173
$this->assertEquals($csv, $this->encoder->encode($value, 'csv', $context));
174174
}
175175

176+
public function testEncodeFormulas()
177+
{
178+
$this->encoder = new CsvEncoder(',', '"', '\\', '.', true);
179+
180+
$this->assertSame(<<<'CSV'
181+
0
182+
" =2+3"
183+
184+
CSV
185+
, $this->encoder->encode(array('=2+3'), 'csv'));
186+
187+
$this->assertSame(<<<'CSV'
188+
0
189+
" -2+3"
190+
191+
CSV
192+
, $this->encoder->encode(array('-2+3'), 'csv'));
193+
194+
$this->assertSame(<<<'CSV'
195+
0
196+
" +2+3"
197+
198+
CSV
199+
, $this->encoder->encode(array('+2+3'), 'csv'));
200+
201+
$this->assertSame(<<<'CSV'
202+
0
203+
" @MyDataColumn"
204+
205+
CSV
206+
, $this->encoder->encode(array('@MyDataColumn'), 'csv'));
207+
}
208+
209+
public function testDoNotEncodeFormulas()
210+
{
211+
$this->assertSame(<<<'CSV'
212+
0
213+
=2+3
214+
215+
CSV
216+
, $this->encoder->encode(array('=2+3'), 'csv'));
217+
218+
$this->assertSame(<<<'CSV'
219+
0
220+
-2+3
221+
222+
CSV
223+
, $this->encoder->encode(array('-2+3'), 'csv'));
224+
225+
$this->assertSame(<<<'CSV'
226+
0
227+
+2+3
228+
229+
CSV
230+
, $this->encoder->encode(array('+2+3'), 'csv'));
231+
232+
$this->assertSame(<<<'CSV'
233+
0
234+
@MyDataColumn
235+
236+
CSV
237+
, $this->encoder->encode(array('@MyDataColumn'), 'csv'));
238+
}
239+
240+
public function testEncodeFormulasWithSettingsPassedInContext()
241+
{
242+
$this->assertSame(<<<'CSV'
243+
0
244+
" =2+3"
245+
246+
CSV
247+
, $this->encoder->encode(array('=2+3'), 'csv', array(
248+
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
249+
)));
250+
251+
$this->assertSame(<<<'CSV'
252+
0
253+
" -2+3"
254+
255+
CSV
256+
, $this->encoder->encode(array('-2+3'), 'csv', array(
257+
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
258+
)));
259+
260+
$this->assertSame(<<<'CSV'
261+
0
262+
" +2+3"
263+
264+
CSV
265+
, $this->encoder->encode(array('+2+3'), 'csv', array(
266+
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
267+
)));
268+
269+
$this->assertSame(<<<'CSV'
270+
0
271+
" @MyDataColumn"
272+
273+
CSV
274+
, $this->encoder->encode(array('@MyDataColumn'), 'csv', array(
275+
CsvEncoder::ESCAPE_FORMULAS_KEY => true,
276+
)));
277+
}
278+
176279
public function testSupportsDecoding()
177280
{
178281
$this->assertTrue($this->encoder->supportsDecoding('csv'));

0 commit comments

Comments
 (0)