Skip to content

Commit 7d31bc4

Browse files
committed
MAGETWO-37209: Excel Formula Injection via CSV/XML export - 2.x
1 parent 4d42108 commit 7d31bc4

File tree

3 files changed

+33
-12
lines changed

3 files changed

+33
-12
lines changed

dev/tests/integration/testsuite/Magento/Framework/Filesystem/File/WriteTest.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public function writeProvider()
8181
['new1.csv', 'w', 'write check', 11],
8282
['new3.csv', 'a', 'write check', 11],
8383
['new5.csv', 'x', 'write check', 11],
84-
['new7.csv', 'c', 'write check', 11]
84+
['new7.csv', 'c', 'write check', 11],
8585
];
8686
}
8787

@@ -117,40 +117,42 @@ public function writeAndReadProvider()
117117
['new2.csv', 'w+', 'write check', 11],
118118
['new4.csv', 'a+', 'write check', 11],
119119
['new6.csv', 'x+', 'write check', 11],
120-
['new8.csv', 'c+', 'write check', 11]
120+
['new8.csv', 'c+', 'write check', 11],
121121
];
122122
}
123123

124124
/**
125125
* Writes one CSV row to the file.
126126
*
127-
* @dataProvider csvProvider
127+
* @dataProvider csvDataProvider
128+
* @param array $expectedData
128129
* @param string $path
129130
* @param array $data
130131
* @param string $delimiter
131132
* @param string $enclosure
132133
*/
133-
public function testWriteCsv($path, array $data, $delimiter = ',', $enclosure = '"')
134+
public function testWriteCsv($expectedData, $path, array $data, $delimiter = ',', $enclosure = '"')
134135
{
135136
$file = $this->getFileInstance($path, 'w+');
136137
$result = $file->writeCsv($data, $delimiter, $enclosure);
137138
$file->seek(0);
138139
$read = $file->readCsv($result, $delimiter, $enclosure);
139140
$file->close();
140141
$this->removeCurrentFile();
141-
$this->assertEquals($data, $read);
142+
$this->assertEquals($expectedData, $read);
142143
}
143144

144145
/**
145146
* Data provider for testWriteCsv
146147
*
147148
* @return array
148149
*/
149-
public function csvProvider()
150+
public function csvDataProvider()
150151
{
151152
return [
152-
['newcsv1.csv', ['field1', 'field2'], ',', '"'],
153-
['newcsv1.csv', ['field1', 'field2'], '%', '@']
153+
[['field1', 'field2'], 'newcsv1.csv', ['field1', 'field2'], ',', '"'],
154+
[['field1', 'field2'], 'newcsv1.csv', ['field1', 'field2'], '%', '@'],
155+
[[' =field1', 'field2'], 'newcsv1.csv', ['=field1', 'field2'], '%', '@'],
154156
];
155157
}
156158

@@ -201,7 +203,7 @@ private function getFileInstance($path, $mode)
201203
[
202204
'path' => $this->currentFilePath,
203205
'driver' => new \Magento\Framework\Filesystem\Driver\File(),
204-
'mode' => $mode
206+
'mode' => $mode,
205207
]
206208
);
207209
}

lib/internal/Magento/Framework/Filesystem/Driver/File.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88
namespace Magento\Framework\Filesystem\Driver;
99

10-
use Magento\Framework\Filesystem\DriverInterface;
1110
use Magento\Framework\Exception\FileSystemException;
11+
use Magento\Framework\Filesystem\DriverInterface;
1212

1313
class File implements DriverInterface
1414
{
@@ -299,7 +299,7 @@ public function copy($source, $destination, DriverInterface $targetDriver = null
299299
[
300300
$source,
301301
$destination,
302-
$this->getWarningMessage()
302+
$this->getWarningMessage(),
303303
]
304304
)
305305
);
@@ -329,7 +329,7 @@ public function symlink($source, $destination, DriverInterface $targetDriver = n
329329
[
330330
$source,
331331
$destination,
332-
$this->getWarningMessage()
332+
$this->getWarningMessage(),
333333
]
334334
)
335335
);
@@ -644,6 +644,16 @@ public function fileWrite($resource, $data)
644644
*/
645645
public function filePutCsv($resource, array $data, $delimiter = ',', $enclosure = '"')
646646
{
647+
/**
648+
* Security enhancement for CSV data processing by Excel-like applications.
649+
* @see https://bugzilla.mozilla.org/show_bug.cgi?id=1054702
650+
*/
651+
foreach ($data as $key => $value) {
652+
if (isset($value[0]) && $value[0] === '=') {
653+
$data[$key] = ' ' . $value;
654+
}
655+
}
656+
647657
$result = @fputcsv($resource, $data, $delimiter, $enclosure);
648658
if (!$result) {
649659
throw new FileSystemException(

lib/internal/Magento/Framework/Filesystem/Io/File.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,15 @@ public function streamWriteCsv(array $row, $delimiter = ',', $enclosure = '"')
177177
if (!$this->_streamHandler) {
178178
return false;
179179
}
180+
/**
181+
* Security enhancement for CSV data processing by Excel-like applications.
182+
* @see https://bugzilla.mozilla.org/show_bug.cgi?id=1054702
183+
*/
184+
foreach ($row as $key => $value) {
185+
if (isset($value[0]) && $value[0] === '=') {
186+
$row[$key] = ' ' . $value;
187+
}
188+
}
180189
return @fputcsv($this->_streamHandler, $row, $delimiter, $enclosure);
181190
}
182191

0 commit comments

Comments
 (0)