Skip to content

Commit 116a3b7

Browse files
zakdmazakdma
committed
MAGETWO-95236: Change regular expression
1 parent 991d785 commit 116a3b7

File tree

3 files changed

+105
-36
lines changed

3 files changed

+105
-36
lines changed

‎app/code/Magento/Sales/Helper/Admin.php

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Sales\Helper;
78

9+
/**
10+
* Sales admin helper.
11+
*/
812
class Admin extends \Magento\Framework\App\Helper\AbstractHelper
913
{
1014
/**
@@ -148,22 +152,15 @@ public function escapeHtmlWithLinks($data, $allowedTags = null)
148152
$links = [];
149153
$i = 1;
150154
$data = str_replace('%', '%%', $data);
151-
$regexp = "/<a\s[^>]*href\s*?=\s*?([\"\']??)([^\" >]*?)\\1[^>]*>(.*)<\/a>/siU";
155+
$regexp = "#(?J)<a"
156+
."(?:(?:\s+(?:(?:href\s*=\s*(['\"])(?<link>.*?)\\1\s*)|(?:\S+\s*=\s*(['\"])(.*?)\\3)\s*)*)|>)"
157+
.">?(?:(?:(?<text>.*?)(?:<\/a\s*>?|(?=<\w))|(?<text>.*)))#si";
152158
while (preg_match($regexp, $data, $matches)) {
153-
//Revert the sprintf escaping
154-
$url = str_replace('%%', '%', $matches[2]);
155-
$text = str_replace('%%', '%', $matches[3]);
156-
//Check for an valid url
157-
if ($url) {
158-
$urlScheme = strtolower(parse_url($url, PHP_URL_SCHEME));
159-
if ($urlScheme !== 'http' && $urlScheme !== 'https') {
160-
$url = null;
161-
}
162-
}
163-
//Use hash tag as fallback
164-
if (!$url) {
165-
$url = '#';
159+
$text = '';
160+
if (!empty($matches['text'])) {
161+
$text = str_replace('%%', '%', $matches['text']);
166162
}
163+
$url = $this->filterUrl($matches['link'] ?? '');
167164
//Recreate a minimalistic secure a tag
168165
$links[] = sprintf(
169166
'<a href="%s">%s</a>',
@@ -178,4 +175,29 @@ public function escapeHtmlWithLinks($data, $allowedTags = null)
178175
}
179176
return $this->escaper->escapeHtml($data, $allowedTags);
180177
}
178+
179+
/**
180+
* Filter the URL for allowed protocols.
181+
*
182+
* @param string $url
183+
* @return string
184+
*/
185+
private function filterUrl(string $url): string
186+
{
187+
if ($url) {
188+
//Revert the sprintf escaping
189+
$url = str_replace('%%', '%', $url);
190+
$urlScheme = parse_url($url, PHP_URL_SCHEME);
191+
$urlScheme = $urlScheme ? strtolower($urlScheme) : '';
192+
if ($urlScheme !== 'http' && $urlScheme !== 'https') {
193+
$url = null;
194+
}
195+
}
196+
197+
if (!$url) {
198+
$url = '#';
199+
}
200+
201+
return $url;
202+
}
181203
}

‎lib/internal/Magento/Framework/Escaper.php

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Framework;
78

89
/**
@@ -32,38 +33,43 @@ class Escaper
3233
*/
3334
private $allowedAttributes = ['id', 'class', 'href', 'target', 'title', 'style'];
3435

36+
/**
37+
* @var string
38+
*/
39+
private static $xssFiltrationPattern =
40+
'/((javascript(\\\\x3a|:|%3A))|(data(\\\\x3a|:|%3A))|(vbscript:))|'
41+
. '((\\\\x6A\\\\x61\\\\x76\\\\x61\\\\x73\\\\x63\\\\x72\\\\x69\\\\x70\\\\x74(\\\\x3a|:|%3A))|'
42+
. '(\\\\x64\\\\x61\\\\x74\\\\x61(\\\\x3a|:|%3A)))/i';
43+
3544
/**
3645
* @var string[]
3746
*/
3847
private $escapeAsUrlAttributes = ['href'];
3948

4049
/**
41-
* Escape string for HTML context. allowedTags will not be escaped, except the following: script, img, embed,
42-
* iframe, video, source, object, audio
50+
* Escape string for HTML context.
51+
*
52+
* AllowedTags will not be escaped, except the following: script, img, embed,
53+
* iframe, video, source, object, audio.
4354
*
4455
* @param string|array $data
4556
* @param array|null $allowedTags
4657
* @return string|array
4758
*/
4859
public function escapeHtml($data, $allowedTags = null)
4960
{
61+
if (!is_array($data)) {
62+
$data = (string)$data;
63+
}
64+
5065
if (is_array($data)) {
5166
$result = [];
5267
foreach ($data as $item) {
5368
$result[] = $this->escapeHtml($item, $allowedTags);
5469
}
5570
} elseif (strlen($data)) {
5671
if (is_array($allowedTags) && !empty($allowedTags)) {
57-
$notAllowedTags = array_intersect(
58-
array_map('strtolower', $allowedTags),
59-
$this->notAllowedTags
60-
);
61-
if (!empty($notAllowedTags)) {
62-
$this->getLogger()->critical(
63-
'The following tag(s) are not allowed: ' . implode(', ', $notAllowedTags)
64-
);
65-
$allowedTags = array_diff($allowedTags, $this->notAllowedTags);
66-
}
72+
$allowedTags = $this->filterProhibitedTags($allowedTags);
6773
$wrapperElementId = uniqid();
6874
$domDocument = new \DOMDocument('1.0', 'UTF-8');
6975
set_error_handler(
@@ -197,7 +203,7 @@ public function escapeHtmlAttr($string, $escapeSingleQuote = true)
197203
if ($escapeSingleQuote) {
198204
return $this->getEscaper()->escapeHtmlAttr((string) $string);
199205
}
200-
return htmlspecialchars($string, ENT_COMPAT, 'UTF-8', false);
206+
return htmlspecialchars((string)$string, ENT_COMPAT, 'UTF-8', false);
201207
}
202208

203209
/**
@@ -278,30 +284,47 @@ public function escapeJsQuote($data, $quote = '\'')
278284
$result[] = $this->escapeJsQuote($item, $quote);
279285
}
280286
} else {
281-
$result = str_replace($quote, '\\' . $quote, $data);
287+
$result = str_replace($quote, '\\' . $quote, (string)$data);
282288
}
283289
return $result;
284290
}
285291

286292
/**
287293
* Escape xss in urls
288-
* Remove `javascript:`, `vbscript:`, `data:` words from url
289294
*
290295
* @param string $data
291296
* @return string
292297
* @deprecated 100.2.0
293298
*/
294299
public function escapeXssInUrl($data)
295300
{
296-
$pattern = '/((javascript(\\\\x3a|:|%3A))|(data(\\\\x3a|:|%3A))|(vbscript:))|'
297-
. '((\\\\x6A\\\\x61\\\\x76\\\\x61\\\\x73\\\\x63\\\\x72\\\\x69\\\\x70\\\\x74(\\\\x3a|:|%3A))|'
298-
. '(\\\\x64\\\\x61\\\\x74\\\\x61(\\\\x3a|:|%3A)))/i';
299-
$result = preg_replace($pattern, ':', $data);
300-
return htmlspecialchars($result, ENT_COMPAT | ENT_HTML5 | ENT_HTML401, 'UTF-8', false);
301+
return htmlspecialchars(
302+
$this->escapeScriptIdentifiers((string)$data),
303+
ENT_COMPAT | ENT_HTML5 | ENT_HTML401,
304+
'UTF-8',
305+
false
306+
);
307+
}
308+
309+
/**
310+
* Remove `javascript:`, `vbscript:`, `data:` words from the string.
311+
*
312+
* @param string $data
313+
* @return string
314+
*/
315+
private function escapeScriptIdentifiers(string $data): string
316+
{
317+
$filteredData = preg_replace(self::$xssFiltrationPattern, ':', $data) ?: '';
318+
if (preg_match(self::$xssFiltrationPattern, $filteredData)) {
319+
$filteredData = $this->escapeScriptIdentifiers($filteredData);
320+
}
321+
322+
return $filteredData;
301323
}
302324

303325
/**
304326
* Escape quotes inside html attributes
327+
*
305328
* Use $addSlashes = false for escaping js that inside html attribute (onClick, onSubmit etc)
306329
*
307330
* @param string $data
@@ -346,4 +369,27 @@ private function getLogger()
346369
}
347370
return $this->logger;
348371
}
372+
373+
/**
374+
* Filter prohibited tags.
375+
*
376+
* @param string[] $allowedTags
377+
* @return string[]
378+
*/
379+
private function filterProhibitedTags(array $allowedTags): array
380+
{
381+
$notAllowedTags = array_intersect(
382+
array_map('strtolower', $allowedTags),
383+
$this->notAllowedTags
384+
);
385+
386+
if (!empty($notAllowedTags)) {
387+
$this->getLogger()->critical(
388+
'The following tag(s) are not allowed: ' . implode(', ', $notAllowedTags)
389+
);
390+
$allowedTags = array_diff($allowedTags, $this->notAllowedTags);
391+
}
392+
393+
return $allowedTags;
394+
}
349395
}

‎lib/internal/Magento/Framework/View/Element/AbstractBlock.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ public function stripTags($data, $allowableTags = null, $allowHtmlEntities = fal
952952
*/
953953
public function escapeUrl($string)
954954
{
955-
return $this->_escaper->escapeUrl($string);
955+
return $this->_escaper->escapeUrl((string)$string);
956956
}
957957

958958
/**
@@ -1152,7 +1152,8 @@ public function getVar($name, $module = null)
11521152

11531153
/**
11541154
* Determine if the block scope is private or public.
1155-
* Returns true if scope is private, false otherwise
1155+
*
1156+
* Returns true if scope is private, false otherwise.
11561157
*
11571158
* @return bool
11581159
*/

0 commit comments

Comments
 (0)