Since SafeMarkup::checkPlain() is now deprecated and possibly intended to be removed along with other SafeMarkup methods, I decided to address this in migrating to RC 1.
In the common use case where D8's Twig templates are used for output, you can dispense with checkPlain() entirely and rely on Twig's autoescaping. That turned out to apply to our simple use of forms to input and render a few text values.
In the original code, checkPlain() was called both to process user-entered values as well as to sanitize those values before they were placed into render arrays. When I typed <script> into a form field, it was incorrectly escaped twice and displayed as <script>
Removing those calls works fine and conforms to what is considered good practice for D8. Potentially unsafe markup is stored as is in the database, but Twig converts it before it is sent to the browser.
The article SafeMarkup methods are removed is extremely useful in how it breaks down the different use cases for checkPlain() and what needs to be done differently for each in D8 in order to prevent unsafe markup from being rendered.
Besides Twig templates, the other use cases are:
- Text placed into a render array by using the #plain_text key
- A mixture of escaped markup with markup not to be escaped
- Non-HTML responses, eg. JSON
The
discussion also applies to the check_plain() function in Drupal 7, for which checkPlain() was a replacement. If
you're starting out with a conversion from D7, the article is a must
read.
Sources:
SafeMarkup::set(), SafeMarkup::checkPlain(), and other methods are removed from Drupal 8 core
https://groups.drupal.org/node/478558
SafeMarkup methods are removed
https://www.drupal.org/node/2549395
Twig autoescape enabled and text sanitization APIs updated
https://www.drupal.org/node/2296163
Dec 19, 2015
Module: "Configuration inspector for Drupal 8"
In my recent post on Missing langcode in configuration schema I discovered that the langcode key in configuration schema is now required by default by Testing module automated tests.
The test results had error messages that said "Uncaught PHP Exception Drupal\Core\Config\Schema\SchemaIncompleteException" and "langcode missing schema".
As a result of that debugging, I got interested in a module called Configuration Inspector for Drupal 8.
Installing the module adds a new tab named Inspect to the page at
/admin/config/development/configuration
That tab shows all of the Configuration Keys on the site, to which core contributes quite a few.
For the settings for our module, after I removed the langcode key from the .schema.yml file, the tab showed that the configuration key had 1 error. Going to the Raw Data page for the key then showed under Configuration Validation,
array (
'optimizely.settings:langcode' => 'missing schema',
)
This is the gist of what the exceptions had said in the results from running the automated tests, but if I had used this config inspector early on to validate the schema, I would have spotted the error sooner rather than after the testing failed.
* * *
The inspector also shows the types and the values of individual items. In our case, this includes a project id number whose value had been submitted through a form and stored programmatically by using the Simple Configuration API.
There was no entry shown for the langcode item, which is defined but had no value. I was able to provide one by adding code in hook_install(), again, by using the Simple Configuration API.
Sources:
Configuration inspector for Drupal 8
https://www.drupal.org/project/config_inspector
Configuration API in Drupal 8
https://www.drupal.org/node/1667894
Configuration schema/metadata
https://www.drupal.org/node/1905070
The test results had error messages that said "Uncaught PHP Exception Drupal\Core\Config\Schema\SchemaIncompleteException" and "langcode missing schema".
As a result of that debugging, I got interested in a module called Configuration Inspector for Drupal 8.
Installing the module adds a new tab named Inspect to the page at
/admin/config/development/configuration
That tab shows all of the Configuration Keys on the site, to which core contributes quite a few.
For the settings for our module, after I removed the langcode key from the .schema.yml file, the tab showed that the configuration key had 1 error. Going to the Raw Data page for the key then showed under Configuration Validation,
array (
'optimizely.settings:langcode' => 'missing schema',
)
This is the gist of what the exceptions had said in the results from running the automated tests, but if I had used this config inspector early on to validate the schema, I would have spotted the error sooner rather than after the testing failed.
* * *
The inspector also shows the types and the values of individual items. In our case, this includes a project id number whose value had been submitted through a form and stored programmatically by using the Simple Configuration API.
There was no entry shown for the langcode item, which is defined but had no value. I was able to provide one by adding code in hook_install(), again, by using the Simple Configuration API.
Sources:
Configuration inspector for Drupal 8
https://www.drupal.org/project/config_inspector
Configuration API in Drupal 8
https://www.drupal.org/node/1667894
Configuration schema/metadata
https://www.drupal.org/node/1905070
Dec 12, 2015
Drupal 8, Beta 14 --> Beta 15: Missing langcode in configuration schema
In migrating from Drupal 8 beta 14 to beta 15, the functionality of the module itself worked fine, but some of the automated tests were failing with error messages that included the following.
The article Fix config schema mentioned a very similar error message and stated "Because of a recent core change all tests are failing". I looked at the patches in that article, but I could not figure out what needed to be added in our case.
Then a search through the core code for the string "langcode:" led me to add langcode: as a key to the .schema.yml file as in the following.
optimizely.settings:
type: mapping
label: 'Optimizely Config Data'
mapping:
optimizely_id:
type: integer
label: 'Optimizely ID Number'
translatable: false
langcode:
type: string
label: 'Language code'
Problem solved. All automated tests passed after this change.
Update: Also see my later post on Module: Configuration Inspector for Drupal 8.
Sources:
Beta 3 --> Beta 4: Configuration schema and metadata
http://optimizely-to-drupal-8.blogspot.com/2014/12/beta-3-beta-4-configuration-schema-and.html
Fix config schema
https://www.drupal.org/node/2547365
All TestBase derived tests now enforce strict configuration schema adherence by default
https://www.drupal.org/node/2391795
Configuration schema/metadata
https://www.drupal.org/node/1905070
Uncaught PHP Exception Drupal\Core\Config\Schema\SchemaIncompleteException: "Schema errors for optimizely.settings with the following errors: optimizely.settings:langcode missing schema" at /var/www/html/opti/core/lib/Drupal/Core/Config/Testing/ConfigSchemaChecker.php line 98In this message, optimizely.settings is the name of a group of configuration settings. It is also a configuration key in a .schema.yml file that is required for automated testing, which I blogged about earlier at Beta 3 --> Beta 4: Configuration schema and metadata.
The article Fix config schema mentioned a very similar error message and stated "Because of a recent core change all tests are failing". I looked at the patches in that article, but I could not figure out what needed to be added in our case.
Then a search through the core code for the string "langcode:" led me to add langcode: as a key to the .schema.yml file as in the following.
optimizely.settings:
type: mapping
label: 'Optimizely Config Data'
mapping:
optimizely_id:
type: integer
label: 'Optimizely ID Number'
translatable: false
langcode:
type: string
label: 'Language code'
Problem solved. All automated tests passed after this change.
Update: Also see my later post on Module: Configuration Inspector for Drupal 8.
Sources:
Beta 3 --> Beta 4: Configuration schema and metadata
http://optimizely-to-drupal-8.blogspot.com/2014/12/beta-3-beta-4-configuration-schema-and.html
Fix config schema
https://www.drupal.org/node/2547365
All TestBase derived tests now enforce strict configuration schema adherence by default
https://www.drupal.org/node/2391795
Configuration schema/metadata
https://www.drupal.org/node/1905070
Nov 10, 2015
"Notice: Undefined index: und in eval() (line . . . . . /modules/php/php.module(80) : eval()'d code)"
On my local instance of a Drupal 7 site, I'd get warnings like the following on almost every page for a custom content type called Poem.
Notice: Undefined index: und in eval() (line 12 of /var/www/html/power-poetry/modules/php/php.module(80) : eval()'d code).
Although most probably innocuous, these warnings were also being logged numerous times in the system log, cluttering it up and making it harder to spot other, significant messages. It was enough of an annoyance that I decided to fix it.
I was able to track this to a snippet of PHP code that is used in a Drupal block of ours. The code is part of the block's Visibility Settings. It determines if the current Poem being rendered satisfies certain criteria or not. If so, the block is made visible.
The offending line turned out to be
$slam_id = $current_slam['und'][0]['target_id'];
The variable $current_slam was often an empty array. So by replacing the line with an additional check, the PHP warnings disappeared.
if (!empty($current_slam)) {
$slam_id = $current_slam['und'][0]['target_id'];
}
The key to finding this quickly was remembering that we have this user-supplied snippet of code that needs to be evaluated at runtime.
- - - - -
While debugging, I wanted to see the type and the value of the variable $current_slam. The following worked to output it to the system log.
watchdog('debug', print_r($current_slam, TRUE), array(),
WATCHDOG_NOTICE);
The second parameter to print_r() determines what kind of return value the function passes back. By default, print_r() returns its status. But by setting the second param to TRUE, the return value is the output string instead.
- - - - -
The site is hosted on Pantheon, which provides this status message:
Source:
Remove the PHP module from Drupal core
https://drupal.org/node/1203886
Notice: Undefined index: und in eval() (line 12 of /var/www/html/power-poetry/modules/php/php.module(80) : eval()'d code).
Although most probably innocuous, these warnings were also being logged numerous times in the system log, cluttering it up and making it harder to spot other, significant messages. It was enough of an annoyance that I decided to fix it.
I was able to track this to a snippet of PHP code that is used in a Drupal block of ours. The code is part of the block's Visibility Settings. It determines if the current Poem being rendered satisfies certain criteria or not. If so, the block is made visible.
The offending line turned out to be
$slam_id = $current_slam['und'][0]['target_id'];
The variable $current_slam was often an empty array. So by replacing the line with an additional check, the PHP warnings disappeared.
if (!empty($current_slam)) {
$slam_id = $current_slam['und'][0]['target_id'];
}
The key to finding this quickly was remembering that we have this user-supplied snippet of code that needs to be evaluated at runtime.
- - - - -
While debugging, I wanted to see the type and the value of the variable $current_slam. The following worked to output it to the system log.
watchdog('debug', print_r($current_slam, TRUE), array(),
WATCHDOG_NOTICE);
The second parameter to print_r() determines what kind of return value the function passes back. By default, print_r() returns its status. But by setting the second param to TRUE, the return value is the output string instead.
- - - - -
The site is hosted on Pantheon, which provides this status message:
- PHP Filter: PHP Filter is enabled! Executable code should never be stored in the database, and support for this feature was removed in Drupal 8 - https://drupal.org/node/1203886Remove all executable code from your content and move it to your codebase.
Source:
Remove the PHP module from Drupal core
https://drupal.org/node/1203886
Nov 3, 2015
Malicious page redirects and Drupal's Filtered-HTML text format
One of the Drupal 7 sites I maintain is www.powerpoetry.org which is a platform for publishing poems and comments about them.
Recently, we heard from an irate poet. When browsing to one of her poems, the page would partially load, then automatically redirect to a movie streaming site that had nothing to do with her poem nor with the site in general. Her poem page somehow got hijacked, and readers could never read her work.
It turned out that all site users, even Anonymous ones, had permissions for the Full HTML Text Format. This allowed comments to be submitted that contained malicious code to redirect the page.
So far, I have found two different techniques that were used to implement redirection.
(1) Use of the <meta> tag with an attribute of http-equiv="refresh". For example,
<meta http-equiv="refresh"
content="2;url=http://scumbags.com/">
where the value of 2 means a delay of two seconds before the refresh.
(2) Use of the onmouseover attribute to execute JavaScript. For example,
<a href="//tinyurl.com/nm8ugh"
onmouseover="document.location='//tinyurl.com/nm8ugh';">
As documented, this attribute and closely related ones are valid for almost all HTML tags, which would make it potentially a big problem where markup is allowed.
If you have good knowledge of HTML, these techniques may be obvious, but for me as a back-end developer they were new.
Because I have direct access to the Drupal database, I was able to use a SQL query such as the following to find malicious comments.
select entity_id, comment_body_value
from field_data_comment_body
where comment_body_value like "%onmouseover%";
- - - - -
To help plug these security holes, we decided to allow only Administrators to have access to all Text Formats. Depending on the needs of your site, you might want to disable the Full HTML format entirely.
For all other users, the Filtered HTML Text Format was permitted, configured using Limit Allowed HTML Tags so that only the following tags are processed.
<p> <br> <em> <strong> <cite> <blockquote> <ul> <ol> <li>
Although not part of this security problem, we purposely disallowed <a> tags, deciding that they are not essential for comments on our site and almost always only appear in spam comments. As part of eliminating links, we also turned off Convert URLs Into Links.
Note that the user can still type in whatever they want, including any kind of HTML. The original text is stored unchanged as the content of the comment.
However, what the Filtered HTML format does is effectively strip out disallowed tags as part of the rendering process.
- - - - -
Even with Filtered HTML, I was concerned that the onmouseover attribute could still be used inside one of the tags that is allowed. What about something like
<p onmouseover="....">
But some testing and some stepping through core code revealed that onmouseover as well as other risky attributes are stripped out of those tags for output even though the tags themselves are processed.
The holes were plugged for these two exploits.
- - - - -
The final thing left to do was to go back and check again the malicious comments, which I had left in the database while carrying out the above steps. I thought that applying Filtered HTML would neutralize them. I was wrong.
In the database table field_data_comment_body there is a column comment_body_format that stores the format in effect when the comment was created. The format apparently is used to render the comment even if it is no longer allowed.
As a result, those comments were still redirecting!
So I used SQL queries to find their entity ids, then deleted them by browsing to, for example, the following path where 30259 is the entity id of a comment.
/comment/30259/edit
or
/comment/30259/delete
The latter path goes directly to the delete confirmation dialog. In some cases I had to use it because loading the edit form for the comment triggered the redirection, so I couldn't even edit it.
- - - - -
Michael Richardson at Pantheon support did an awesome job of doing the initial troubleshooting when I was at a loss as to where to start. It was he who uncovered the use of onmouseover on our site. Thanks, Michael!
Sources:
What is the Meta Refresh Tag?
http://webdesign.about.com/od/metataglibraries/a/aa080300a.htm
HTML onmouseover Event Attribute
http://www.w3schools.com/tags/ev_onmouseover.asp
Drupal Text Formats and Filters Tutorial
https://www.hostknox.com/tutorials/drupal/formats-and-filters
Text Filters and Input Formats
https://www.drupal.org/node/213156
Recently, we heard from an irate poet. When browsing to one of her poems, the page would partially load, then automatically redirect to a movie streaming site that had nothing to do with her poem nor with the site in general. Her poem page somehow got hijacked, and readers could never read her work.
It turned out that all site users, even Anonymous ones, had permissions for the Full HTML Text Format. This allowed comments to be submitted that contained malicious code to redirect the page.
So far, I have found two different techniques that were used to implement redirection.
(1) Use of the <meta> tag with an attribute of http-equiv="refresh". For example,
<meta http-equiv="refresh"
content="2;url=http://scumbags.com/">
where the value of 2 means a delay of two seconds before the refresh.
(2) Use of the onmouseover attribute to execute JavaScript. For example,
<a href="//tinyurl.com/nm8ugh"
onmouseover="document.location='//tinyurl.com/nm8ugh';">
As documented, this attribute and closely related ones are valid for almost all HTML tags, which would make it potentially a big problem where markup is allowed.
If you have good knowledge of HTML, these techniques may be obvious, but for me as a back-end developer they were new.
Because I have direct access to the Drupal database, I was able to use a SQL query such as the following to find malicious comments.
select entity_id, comment_body_value
from field_data_comment_body
where comment_body_value like "%onmouseover%";
- - - - -
To help plug these security holes, we decided to allow only Administrators to have access to all Text Formats. Depending on the needs of your site, you might want to disable the Full HTML format entirely.
For all other users, the Filtered HTML Text Format was permitted, configured using Limit Allowed HTML Tags so that only the following tags are processed.
<p> <br> <em> <strong> <cite> <blockquote> <ul> <ol> <li>
Although not part of this security problem, we purposely disallowed <a> tags, deciding that they are not essential for comments on our site and almost always only appear in spam comments. As part of eliminating links, we also turned off Convert URLs Into Links.
Note that the user can still type in whatever they want, including any kind of HTML. The original text is stored unchanged as the content of the comment.
However, what the Filtered HTML format does is effectively strip out disallowed tags as part of the rendering process.
- - - - -
Even with Filtered HTML, I was concerned that the onmouseover attribute could still be used inside one of the tags that is allowed. What about something like
<p onmouseover="....">
But some testing and some stepping through core code revealed that onmouseover as well as other risky attributes are stripped out of those tags for output even though the tags themselves are processed.
The holes were plugged for these two exploits.
- - - - -
The final thing left to do was to go back and check again the malicious comments, which I had left in the database while carrying out the above steps. I thought that applying Filtered HTML would neutralize them. I was wrong.
In the database table field_data_comment_body there is a column comment_body_format that stores the format in effect when the comment was created. The format apparently is used to render the comment even if it is no longer allowed.
As a result, those comments were still redirecting!
So I used SQL queries to find their entity ids, then deleted them by browsing to, for example, the following path where 30259 is the entity id of a comment.
/comment/30259/edit
or
/comment/30259/delete
The latter path goes directly to the delete confirmation dialog. In some cases I had to use it because loading the edit form for the comment triggered the redirection, so I couldn't even edit it.
- - - - -
Michael Richardson at Pantheon support did an awesome job of doing the initial troubleshooting when I was at a loss as to where to start. It was he who uncovered the use of onmouseover on our site. Thanks, Michael!
Sources:
What is the Meta Refresh Tag?
http://webdesign.about.com/od/metataglibraries/a/aa080300a.htm
HTML onmouseover Event Attribute
http://www.w3schools.com/tags/ev_onmouseover.asp
Drupal Text Formats and Filters Tutorial
https://www.hostknox.com/tutorials/drupal/formats-and-filters
Text Filters and Input Formats
https://www.drupal.org/node/213156
Subscribe to:
Posts (Atom)