Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Follow-up to #2501639: Remove SafeMarkup::set in drupal_check_module()
Problem/Motivation
FieldPluginBase:advancedRender calls SafeMarkup::set() which is meant to be for internal use only.
Proposed resolution
- Remove the call by refactoring the code.
If refactoring is not possible, thoroughly document where the string is coming from and why it is safe, and why SafeMarkup::set() is required.
Remaining tasks
- Evaluate whether the string can be refactored to one of the formats outlined in this change record: https://www.drupal.org/node/2311123
- Identify whether there is existing automated test coverage for the sanitization of the string. If there is, list the test in the issue summary. If there isn't, add an automated test for it.
- If the string cannot be refactored, the SafeMarkup::set() usage needs to be thoroughly audited and documented.
Manual testing steps (for XSS and double escaping)
Do these steps both with HEAD and with the patch applied:
- Clean install of Drupal 8.
- Compare the output above in HEAD and with the patch applied. Confirm that there is no double-escaping.
- If there is any user or calling code input in the string, submit
alert('XSS');and ensure that it is sanitized.
User interface changes
N/A
API changes
N/A
Comments
Comment #1
kgoel CreditAttribution: kgoel at Forum One commentedComment #2
kgoel CreditAttribution: kgoel at Forum One commentedComment #3
kgoel CreditAttribution: kgoel at Forum One commentedClosed https://www.drupal.org/node/2280961 as duplicate, this issue addresses the remaining SafeMarkup::set in FieldPluginBase.php
Comment #4
chx CreditAttribution: chx commentedComment #5
chx CreditAttribution: chx commentedComment #6
RavindraSingh CreditAttribution: RavindraSingh as a volunteer and at Srijan | A Material+ Company commentedIssue can be closed as it has been mentioned as a duplicate of #2501931: Remove SafeMarkup::set in twig_render_template() and ThemeManager and FieldPluginBase:advancedRender
Comment #7
alimac CreditAttribution: alimac at University of Illinois at Chicago commentedComment #8
alimac CreditAttribution: alimac at University of Illinois at Chicago commented