Commit dfb0685
committed
fix: escapeUrl("0") should return "0", not ""
previously, we assumed that when removing XSS-like script URLs, the only
non-truthy result would be a failure in preg_replace, or an empty
string. A value of "0" would therefor pass through the script-tag
check, but then be converted into the empty string because it is
non-truthy.
instead, we now specifically test for those values which we want to
convert into an empty string, and do so immediately. This allows the
value of "0" to remain intact when passed through a escape methods such
as encodeUrlParam(...) or escapeUrl(...)
Arguably, this is not the "real" solution. Theoretically, this happens
because of the expectation that escapeXssInUrl should only be called
on "full" URLs, and is intended to remove URLs such as
"javascript:alert('I am an XSS attack')". However, escapeXssInUrl is
used by escapeUrl, which is in-turn used by encodeUrlParam. I believe
this to potentially be a bug in encodeUrlParam, which (by its name)
seems to be intended to actually do something similar to PHP's builtin
urlencode(...) function.
I consider this to be a separate issue, however, and this change would
both fix the encodeUrlParam case for this specific issue, while not
negatively-impacting the pathological case of the string "0" being
passed to escapeUrl directly.1 parent cac46e0 commit dfb0685
File tree
2 files changed
+14
-2
lines changed- lib/internal/Magento/Framework
- Test/Unit
2 files changed
+14
-2
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
335 | 335 | | |
336 | 336 | | |
337 | 337 | | |
338 | | - | |
339 | | - | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
340 | 348 | | |
341 | 349 | | |
342 | 350 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
343 | 343 | | |
344 | 344 | | |
345 | 345 | | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
346 | 350 | | |
347 | 351 | | |
348 | 352 | | |
| |||
0 commit comments