#352121: locale_string_is_safe() is not suitable for user provided text in non-default textgroups

From:  <>


---

 includes/locale.inc                     |    8 ++++++--
 modules/locale/locale.test              |   15 +++++++++++++--
 modules/locale/tests/locale_test.info   |    8 ++++++++
 modules/locale/tests/locale_test.module |   17 +++++++++++++++++
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git includes/locale.inc includes/locale.inc
index 6b4e4f6..891f69d 100644
--- includes/locale.inc
+++ includes/locale.inc
@@ -856,8 +856,10 @@ function locale_string_is_safe($string) {
  * Validate string editing form submissions.
  */
 function locale_translate_edit_form_validate($form, &$form_state) {
+  // Locale string check is needed for default textgroup only.
+  $safe_check_needed = $form_state['values']['textgroup'] == 'default';
   foreach ($form_state['values']['translations'] as $key => $value) {
-    if (!locale_string_is_safe($value)) {
+    if ($safe_check_needed && !locale_string_is_safe($value)) {
       form_set_error('translations', t('The submitted string contains disallowed HTML: %string', array('%string' => $value)));
       watchdog('locale', 'Attempted submission of a translation string with disallowed HTML: %string', array('%string' => $value), WATCHDOG_WARNING);
     }
@@ -1349,7 +1351,9 @@ function _locale_import_one_string_db(&$report, $langcode, $source, $translation
 
   if (!empty($translation)) {
     // Skip this string unless it passes a check for dangerous code.
-    if (!locale_string_is_safe($translation)) {
+    // Text groups other than default still can contain HTML tags
+    // (i.e. translatable blocks).
+    if ($textgroup == "default" && !locale_string_is_safe($translation)) {
       $report['skips']++;
       $lid = 0;
     }
diff --git modules/locale/locale.test modules/locale/locale.test
index 623a7ed..84cae10 100644
--- modules/locale/locale.test
+++ modules/locale/locale.test
@@ -191,7 +191,7 @@ class LocaleImportFunctionalTest extends DrupalWebTestCase {
   protected $admin_user = NULL;
 
   function setUp() {
-    parent::setUp('locale');
+    parent::setUp('locale', 'locale_test');
 
     $this->admin_user = $this->drupalCreateUser(array('administer languages', 'translate interface', 'access administration pages'));
     $this->drupalLogin($this->admin_user);
@@ -216,7 +216,7 @@ class LocaleImportFunctionalTest extends DrupalWebTestCase {
     // The importation should have create 7 strings.
     $this->assertRaw(t('The translation was successfully imported. There are %number newly created translated strings, %update strings were updated and %delete strings were removed.', array('%number' => 7, '%update' => 0, '%delete' => 0)), t('The translation file was successfully imported'));
 
-    // Try importing a .po file with script.
+    // Try importing a .po file with invalid tags in the default text group.
     $name = tempnam(file_directory_temp(), "po_");
     file_put_contents($name, $this->getBadPoFile());
     $this->drupalPost('admin/build/translate/import', array(
@@ -229,6 +229,17 @@ class LocaleImportFunctionalTest extends DrupalWebTestCase {
     $skip_message = format_plural(2, 'One translation string was skipped because it contains disallowed HTML.', '@count translation strings were skipped because they contain disallowed HTML.');
     $this->assertRaw($skip_message, t('Unsafe strings were skipped.'));
 
+    // Try importing a .po file with invalid tags in a non default text group.
+    $name = tempnam(file_directory_temp(), "po_");
+    file_put_contents($name, $this->getBadPoFile());
+    $this->drupalPost('admin/build/translate/import', array(
+      'langcode' => 'fr',
+      'files[file]' => $name,
+      'group' => 'custom',
+    ), t('Import'));
+    unlink($name);
+    // The importation should have created 3 strings.
+    $this->assertRaw(t('The translation was successfully imported. There are %number newly created translated strings, %update strings were updated and %delete strings were removed.', array('%number' => 3, '%update' => 0, '%delete' => 0)), t('The translation file was successfully imported.'));
   }
 
   /**
diff --git modules/locale/tests/locale_test.info modules/locale/tests/locale_test.info
new file mode 100644
index 0000000..d3602c1
--- /dev/null
+++ modules/locale/tests/locale_test.info
@@ -0,0 +1,8 @@
+; $Id $
+name = "Locale Test"
+description = "Support module for the locale layer tests."
+core = 7.x
+package = Testing
+files[] = locale_test.module
+version = VERSION
+hidden = TRUE
diff --git modules/locale/tests/locale_test.module modules/locale/tests/locale_test.module
new file mode 100644
index 0000000..45b19e6
--- /dev/null
+++ modules/locale/tests/locale_test.module
@@ -0,0 +1,17 @@
+<?php
+// $Id $
+
+/**
+ * @file
+ * Mock module for locale layer tests.
+ */
+
+/**
+ * Implementation of hook_locale().
+ */
+function locale_test_locale($op = 'groups') {
+  switch ($op) {
+    case 'groups':
+      return array('custom' => t('Custom'));
+  }
+}