Problem/Motivation

This was flagged by automated security scanners, but even at it worst score it stays in "Less Critical", so per https://www.drupal.org/psa-2023-07-12 I will file this public.

A user who is allowed only to manage URL redirects can store HTML in a redirect's source path that renders as live markup on the redirect administration list for any administrator. It is filtered through XSS protection, so the harm is minimal.

Example of injecting the HTML for a cat image

The permission is not behind restrict_access.

Steps to reproduce

1. Add the following in from: newsletter"><img src=https://t3.ftcdn.net/jpg/07/31/60/94/360_F_731609447_8IpYKdmJ4iwWiPpHCENvVyDlthA4i0fu.jpg>

Proposed resolution

Render the value with #plain_text

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork redirect-3624657

Command icon Show commands

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

marcus_johansson created an issue. See original summary.

marcus_johansson’s picture

Issue summary: View changes
marcus_johansson’s picture

Status: Active » Needs review

I've added that it renders the source via plain_text instead. I can't see any case where you need to render it as markup?

The two tests are AI generated - one functional that the markup is not rendered and one kernel. If that is overkill, feel free to remove them. There are errors in CI, but they are pre-exisiting.

marcus_johansson’s picture

StatusFileSize
new30.93 KB

Oh, also same cat image after MR:

kristen pol’s picture

Status: Needs review » Needs work

Thanks 🙏 I’m on my phone so can’t update it right now but it’s failing phpcs so moving to needs work

kristen pol’s picture

Assigned: Unassigned » kristen pol

On computer now. I do see failed pipelines recently:

https://git.drupalcode.org/project/redirect/-/jobs/12254973
https://git.drupalcode.org/project/redirect/-/jobs/12254974

so I wonder if this unrelated to this MR. I'll check.

kristen pol’s picture

Status: Needs work » Needs review

Confirmed the phpcs/phpstan failures are pre-existing


A TOTAL OF 10 ERRORS AND 0 WARNINGS WERE FOUND IN 114 FILES
FILE: tests/src/Unit/RouteNormalizerRequestSubscriberTest.php
------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
------------------------------------------------------------------------------------------------------------------------
108 | ERROR | The array declaration extends to column 127 (the limit is 120). The array content should be split up
| | over multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
110 | ERROR | The array declaration extends to column 122 (the limit is 120). The array content should be split up
| | over multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
239 | ERROR | The array declaration extends to column 148 (the limit is 120). The array content should be split up
| | over multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: tests/src/Functional/RedirectUILanguageTest.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
26 | ERROR | The array declaration extends to column 123 (the limit is 120). The array content should be split up over
| | multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: src/Form/RedirectSettingsForm.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
59 | ERROR | The array declaration extends to column 426 (the limit is 120). The array content should be split up over
| | multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: src/Form/RedirectDeleteForm.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
20 | ERROR | The array declaration extends to column 203 (the limit is 120). The array content should be split up over
| | multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: src/Exception/RedirectLoopException.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
35 | ERROR | The array declaration extends to column 126 (the limit is 120). The array content should be split up over
| | multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: src/EventSubscriber/RedirectRequestSubscriber.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
154 | ERROR | The array declaration extends to column 155 (the limit is 120). The array content should be split up
| | over multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: src/Entity/Redirect.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
414 | ERROR | The array declaration extends to column 268 (the limit is 120). The array content should be split up
| | over multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
FILE: modules/redirect_404/src/Form/RedirectFix404Form.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
157 | ERROR | The array declaration extends to column 134 (the limit is 120). The array content should be split up
| | over multiple lines (Drupal.Arrays.Array.LongLineDeclaration)
------------------------------------------------------------------------------------------------------------------------
Time: 1.31 secs; Memory: 14MB
PHP CODE SNIFFER REPORT SUMMARY
----------------------------------------------------------------------------
FILE ERRORS WARNINGS
----------------------------------------------------------------------------
modules/redirect_404/src/Form/RedirectFix404Form.php 1 0
src/Entity/Redirect.php 1 0
src/EventSubscriber/RedirectRequestSubscriber.php 1 0
src/Exception/RedirectLoopException.php 1 0
src/Form/RedirectDeleteForm.php 1 0
src/Form/RedirectSettingsForm.php 1 0
tests/src/Functional/RedirectUILanguageTest.php 1 0
tests/src/Unit/RouteNormalizerRequestSubscriberTest.php 3 0
----------------------------------------------------------------------------
A TOTAL OF 10 ERRORS AND 0 WARNINGS WERE FOUND IN 114 FILES
----------------------------------------------------------------------------


[ERROR] Found 9 errors
------ -----------------------------------------------------------------------
Line redirect.install
------ -----------------------------------------------------------------------
149 Avoid calling Symfony\Component\Yaml\Yaml::parse() directly. Use
\Drupal\Component\Serialization\Yaml::decode() instead, which handles
exceptions consistently and applies the correct parse flags.
🪪 drupal.symfonyYamlParse
------ -----------------------------------------------------------------------
------ ---------------------------------------------------------------------
Line src/Form/RedirectDeleteMultipleForm.php
------ ---------------------------------------------------------------------
62 Storing entity storage as a class property is not recommended. Call
Drupal\Core\Entity\EntityTypeManagerInterface::getStorage() at the
call-site instead.
🪪 drupal.entityStoragePropertyAssignment
💡 See
https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal
------ ---------------------------------------------------------------------
------ --------------------------------------------
Line src/Hook/RedirectEntityHooks.php
------ --------------------------------------------
64 No error to ignore is reported on line 64.
🪪 ignore.unmatchedLine (non-ignorable)
------ --------------------------------------------
------ ---------------------------------------------------------------------
Line tests/src/Functional/RedirectCacheTest.php
------ ---------------------------------------------------------------------
47 Storing entity storage as a class property is not recommended. Call
Drupal\Core\Entity\EntityTypeManagerInterface::getStorage() at the
call-site instead.
🪪 drupal.entityStoragePropertyAssignment
💡 See
https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal
------ ---------------------------------------------------------------------
------ ---------------------------------------------------------------------
Line tests/src/Functional/RedirectRelativeUrlTest.php
------ ---------------------------------------------------------------------
49 Storing entity storage as a class property is not recommended. Call
Drupal\Core\Entity\EntityTypeManagerInterface::getStorage() at the
call-site instead.
🪪 drupal.entityStoragePropertyAssignment
💡 See
https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal
------ ---------------------------------------------------------------------
------ ---------------------------------------------------------------------
Line tests/src/Functional/RedirectUITest.php
------ ---------------------------------------------------------------------
93 Storing entity storage as a class property is not recommended. Call
Drupal\Core\Entity\EntityTypeManagerInterface::getStorage() at the
call-site instead.
🪪 drupal.entityStoragePropertyAssignment
💡 See
https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal
269 No error to ignore is reported on line 269.
🪪 ignore.unmatchedLine (non-ignorable)
------ ---------------------------------------------------------------------
------ ---------------------------------------------------------------------
Line tests/src/FunctionalJavascript/RedirectJavascriptTest.php
------ ---------------------------------------------------------------------
73 Storing entity storage as a class property is not recommended. Call
Drupal\Core\Entity\EntityTypeManagerInterface::getStorage() at the
call-site instead.
🪪 drupal.entityStoragePropertyAssignment
💡 See
https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal
------ ---------------------------------------------------------------------
------ ---------------------------------------------------------------------
Line tests/src/Kernel/RedirectAPITest.php
------ ---------------------------------------------------------------------
52 Storing entity storage as a class property is not recommended. Call
Drupal\Core\Entity\EntityTypeManagerInterface::getStorage() at the
call-site instead.
🪪 drupal.entityStoragePropertyAssignment
💡 See
https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal
------ ---------------------------------------------------------------------
[ERROR] Found 9 errors

kristen pol’s picture

Status: Needs review » Reviewed & tested by the community

I've tested and reviewed the code and it looks good to me.

See notes on the MR.

kristen pol’s picture

I've merged the MR.

I'm not sure about creating a release because there is also this Drupal 12 compatibility change since the last release:

commit 5ae862fa2946d40f4ec08eac3a950061000f060e (redirect-3624657/HEAD, redirect-3624657/8.x-1.x)
Author: Sascha Grossenbacher <5019-berdir@users.noreply.drupalcode.org>
Date: Sun Jul 5 22:30:04 2026 +0000

task: #3602388 Drupal 12 compatibility

By: berdir

kristen pol’s picture

Assigned: kristen pol » Unassigned
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.