Closed (fixed)
Project:
Easy Breadcrumb
Version:
2.x-dev
Component:
Code
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
13 May 2024 at 09:39 UTC
Updated:
11 Jul 2024 at 14:24 UTC
Jump to comment: Most recent
When malicious data is passed in the url, invalid json+ld data is generated. This could possibly lead to security issues?
This can be reproduced by requesting a page using the following wget command:
wget http://localhost/index.php/filter/WOORD%5c%22}]DitMagNietKunnen,alert(1)" --content-on-error
Which leads to invalid json+ld data in ListItem 3:
<script type="application/ld+json">
{
"@context": "https://schema.org",
"@type": "BreadcrumbList",
"itemListElement": [{
"@type": "ListItem",
"position": "1",
"name": "Home",
"item": "http://localhost/index.php/"
},{
"@type": "ListItem",
"position": "2",
"name": "Filter",
"item": "http://localhost/index.php/"
},{
"@type": "ListItem",
"position": "3",
"name": "WOORD\\"}]DitMagNietKunnen,alert(1)",
"item": "http://localhost/index.php/"
}]}
</script>
I don't know if there is a specific reason why the EasyBreadcrumbStructuredDataJsonLd class generates the json+ld data by concatenating strings, but I think that should be replaced by json_encode. I'll create a MR for that shortly.
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
ruuds commentedThe test failures are not related. The tests are broken anyway.
Comment #4
spuky commentedlooks good to me json encode is the for sure the better way to go...
Comment #5
norman.lolIsn't
$positionalways1now?Comment #6
norman.lolA single
json_encode($value)without any flag also is not enough. You need at leastjson_encode($value, JSON_UNESCAPED_SLASHES)to prevent slashes in the URLs being escaped.Comment #7
norman.lolComment #8
norman.lolComment #9
ruuds commentedGreat suggestion, looks good to me.
Comment #10
norman.lolComment #11
spuky commentedComment #14
spuky commentedComment #15
norman.lolYeah, I think we've just fixed the related issue as well. json_encode together with the constants we use makes it quite safe against XSS.