- Issue created by @iheb.attia
- ๐ซ๐ทFrance berramou
I did a quick review, I didn't find any serious security issue but still need to sanitize alias and original parts before replacing, and maybe add
$node->access()
to check node access.
Something like:if (is_numeric($nid)) { $node = $this->entityTypeManager->getStorage('node')->load($nid); if ($node instanceof Node && $node->access()) { $language = $this->languageManager->getLanguage($langcode); $node_url = $node->toUrl('canonical', ['language' => $language]); $alias = $node_url->toString(); if ($alias !== "/node/$nid") { // Sanitize alias and original parts before replacing $safe_original = Xss::filter($original); $safe_alias = Html::escape($alias); $safe_url = Html::escape($url); return str_replace($safe_url, $safe_alias, $safe_original); } } } return Html::escape($original);
- ๐ซ๐ทFrance iheb.attia
berramou #3 ๐ Project application for Security Advisory coverage Active
At this stage of the process, the alias and original strings are already sanitized. The plugin filter is executed after the node is built and its access is checked, so we can be confident that the node access logic has already been enforced upstream.Also, note that the final output is passed through Xss::filter() after rendering, which strips any remaining HTML tags. So even if the alias or original contained HTML at some point, it will be removed before display, ensuring XSS protection.
Let me know if you still have concerns, but from what I see, the current flow already handles both access control and sanitization appropriately
- ๐ซ๐ทFrance berramou
Thank you @iheb.attia for your contribution and explications.
It's clear, since there is no other security issue, i will move this to fixed! - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
These issues are fixed only when the applicant get the role that allows to opt projects into security advisory coverage. That can only be done by project moderators.