Commit 8802e0d3 authored by Dries's avatar Dries

- Patch #276111 by pwolanin, Gabor et al: validate translation strings on import.

parent 8ad5cba9
......@@ -673,7 +673,7 @@ function locale_translate_import_form_submit($form, &$form_state) {
}
else {
drupal_set_message(t('File to import not found.'), 'error');
return 'admin/build/translate/import';
$form_state['redirect'] = 'admin/build/translate/import';
}
$form_state['redirect'] = 'admin/build/translate';
......@@ -833,8 +833,39 @@ function locale_translate_edit_form(&$form_state, $lid) {
return $form;
}
/**
* Check that a string is safe to be added or imported as a translation.
*
* This test can be used to detect possibly bad translation strings. It should
* not have any false positives. But it is only a test, not a transformation,
* as it destroys valid HTML. We cannot reliably filter translation strings
* on inport becuase some strings are irreversibly corrupted. For example,
* a & in the translation would get encoded to & by filter_xss()
* before being put in the database, and thus would be displayed incorrectly.
*
* The allowed tag list is like filter_xss_admin(), but omitting div and img as
* not needed for translation and likely to cause layout issues (div) or a
* possible attack vector (img).
*/
function locale_string_is_safe($string) {
return decode_entities($string) == decode_entities(filter_xss($string, array('a', 'abbr', 'acronym', 'address', 'b', 'bdo', 'big', 'blockquote', 'br', 'caption', 'cite', 'code', 'col', 'colgroup', 'dd', 'del', 'dfn', 'dl', 'dt', 'em', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hr', 'i', 'ins', 'kbd', 'li', 'ol', 'p', 'pre', 'q', 'samp', 'small', 'span', 'strong', 'sub', 'sup', 'table', 'tbody', 'td', 'tfoot', 'th', 'thead', 'tr', 'tt', 'ul', 'var')));
}
/**
* Validate string editing form submissions.
*/
function locale_translate_edit_form_validate($form, &$form_state) {
foreach ($form_state['values']['translations'] as $key => $value) {
if (!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);
}
}
}
/**
* Process string editing form submissions.
*
* Saves all translations of one string submitted from a form.
*/
function locale_translate_edit_form_submit($form, &$form_state) {
......@@ -1012,7 +1043,7 @@ function _locale_import_po($file, $langcode, $mode, $group = NULL) {
}
// Get status information on import process.
list($headerdone, $additions, $updates, $deletes) = _locale_import_one_string('db-report');
list($headerdone, $additions, $updates, $deletes, $skips) = _locale_import_one_string('db-report');
if (!$headerdone) {
drupal_set_message(t('The translation file %filename appears to have a missing or malformed header.', array('%filename' => $file->filename)), 'error');
......@@ -1027,6 +1058,11 @@ function _locale_import_po($file, $langcode, $mode, $group = NULL) {
drupal_set_message(t('The translation was successfully imported. There are %number newly created translated strings, %update strings were updated and %delete strings were removed.', array('%number' => $additions, '%update' => $updates, '%delete' => $deletes)));
watchdog('locale', 'Imported %file into %locale: %number new strings added, %update updated and %delete removed.', array('%file' => $file->filename, '%locale' => $langcode, '%number' => $additions, '%update' => $updates, '%delete' => $deletes));
if ($skips) {
$skip_message = format_plural($skips, 'One translation string was skipped because it contains disallowed HTML.', '@count translation strings were skipped because they contain disallowed HTML.');
drupal_set_message($skip_message);
watchdog('locale', $skip_message, NULL, WATCHDOG_WARNING);
}
return TRUE;
}
......@@ -1216,7 +1252,7 @@ function _locale_import_message($message, $file, $lineno = NULL) {
* Text group to import PO file into (eg. 'default' for interface translations)
*/
function _locale_import_one_string($op, $value = NULL, $mode = NULL, $lang = NULL, $file = NULL, $group = 'default') {
static $report = array(0, 0, 0);
static $report = array('additions' => 0, 'updates' => 0, 'deletes' => 0, 'skips' => 0);
static $headerdone = FALSE;
static $strings = array();
......@@ -1232,7 +1268,7 @@ function _locale_import_one_string($op, $value = NULL, $mode = NULL, $lang = NUL
// Called at end of import to inform the user
case 'db-report':
return array($headerdone, $report[0], $report[1], $report[2]);
return array($headerdone, $report['additions'], $report['updates'], $report['deletes'], $report['skips']);
// Store the string we got in the database.
case 'db-store':
......@@ -1311,19 +1347,24 @@ function _locale_import_one_string_db(&$report, $langcode, $source, $translation
$lid = db_result(db_query("SELECT lid FROM {locales_source} WHERE source = '%s' AND textgroup = '%s'", $source, $textgroup));
if (!empty($translation)) {
if ($lid) {
// Skip this string unless it passes a check for dangerous code.
if (!locale_string_is_safe($translation)) {
$report['skips']++;
$lid = 0;
}
elseif ($lid) {
// We have this source string saved already.
db_query("UPDATE {locales_source} SET location = '%s' WHERE lid = %d", $location, $lid);
$exists = (bool) db_result(db_query("SELECT lid FROM {locales_target} WHERE lid = %d AND language = '%s'", $lid, $langcode));
if (!$exists) {
// No translation in this language.
db_query("INSERT INTO {locales_target} (lid, language, translation, plid, plural) VALUES (%d, '%s', '%s', %d, %d)", $lid, $langcode, $translation, $plid, $plural);
$report[0]++;
$report['additions']++;
}
elseif ($mode == LOCALE_IMPORT_OVERWRITE) {
// Translation exists, only overwrite if instructed.
db_query("UPDATE {locales_target} SET translation = '%s', plid = %d, plural = %d WHERE language = '%s' AND lid = %d", $translation, $plid, $plural, $langcode, $lid);
$report[1]++;
$report['updates']++;
}
}
else {
......@@ -1331,13 +1372,13 @@ function _locale_import_one_string_db(&$report, $langcode, $source, $translation
db_query("INSERT INTO {locales_source} (location, source, textgroup) VALUES ('%s', '%s', '%s')", $location, $source, $textgroup);
$lid = db_result(db_query("SELECT lid FROM {locales_source} WHERE source = '%s' AND textgroup = '%s'", $source, $textgroup));
db_query("INSERT INTO {locales_target} (lid, language, translation, plid, plural) VALUES (%d, '%s', '%s', %d, %d)", $lid, $langcode, $translation, $plid, $plural);
$report[0]++;
$report['additions']++;
}
}
elseif ($mode == LOCALE_IMPORT_OVERWRITE) {
// Empty translation, remove existing if instructed.
db_query("DELETE FROM {locales_target} WHERE language = '%s' AND lid = %d AND plid = %d AND plural = %d", $translation, $langcode, $lid, $plid, $plural);
$report[2]++;
$report['deletes']++;
}
return $lid;
......
......@@ -4,8 +4,8 @@
class LocaleTestCase extends DrupalWebTestCase {
function getInfo() {
return array(
'name' => t('String translate'),
'description' => 'Adds a new locale and translates its name',
'name' => t('String translate and validate'),
'description' => 'Adds a new locale and translates its name. Checks the validation of translation strings.',
'group' => 'Locale',
);
}
......@@ -110,6 +110,67 @@ class LocaleTestCase extends DrupalWebTestCase {
$this->drupalPost('admin/build/translate/search', $search, t('Search'));
$this->assertNoText($name, 'Search now can not find the name');
}
function testLocaleStringTest() {
global $base_url;
// User to add language and strings
$admin_user = $this->drupalCreateUser(array('administer languages', 'access administration pages', 'translate interface'));
$this->drupalLogin($admin_user);
$langcode = str_replace('simpletest_', 'si-', $this->randomName(6));
// The English name for the language. This will be translated.
$name = $this->randomName(16);
// The native name for the language.
$native = $this->randomName(16);
// The domain prefix. Not tested yet.
$prefix = strtolower(str_replace('si-', '', $langcode));
// This is the language indicator on the translation search screen for
// untranslated strings. Copied straight from locale.inc.
$language_indicator = "<em class=\"locale-untranslated\">$langcode</em> ";
// These will be the invalid translations of $name.
$key = $this->randomName(16);
$bad_translations[$key] = "<script>alert('xss');</script>" . $key;
$key = $this->randomName(16);
$bad_translations[$key] = '<img SRC="javascript:alert(\'xss\');">' . $key;
$key = $this->randomName(16);
$bad_translations[$key] = '<<SCRIPT>alert("xss");//<</SCRIPT>' . $key;
$key = $this->randomName(16);
$bad_translations[$key] ="<BODY ONLOAD=alert('xss')>" . $key;
// Add language.
$edit = array (
'langcode' => $langcode,
'name' => $name,
'native' => $native,
'prefix' => $prefix,
'direction' => '0',
);
$this->drupalPost('admin/settings/language/add', $edit, t('Add custom language'));
// Add string.
t($name, array(), $langcode);
// Reset locale cache.
$search = array (
'string' => $name,
'language' => 'all',
'translation' => 'all',
'group' => 'all',
);
$this->drupalPost('admin/build/translate/search', $search, t('Search'));
// Find the edit path
$content = $this->drupalGetContent();
$this->assertTrue(preg_match('@(admin/build/translate/edit/[0-9]+)@', $content, $matches), t('Found the edit path'));
$path = $matches[0];
foreach ($bad_translations as $key => $translation) {
$edit = array (
"translations[$langcode]" => $translation,
);
$this->drupalPost($path, $edit, t('Save translations'));
// Check for a form error on the textarea.
$form_class = $this->xpath('//form[@id="locale-translate-edit-form"]//textarea/@class');
$this->assertNotIdentical(FALSE, strpos($form_class[0], 'error'), t('The string was rejected as unsafe.'));
$this->assertNoText(t('The string has been saved.'), t('The string was not saved.'));
}
}
}
/**
......@@ -154,6 +215,20 @@ 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.
$name = tempnam(file_directory_temp(), "po_");
file_put_contents($name, $this->getBadPoFile());
$this->drupalPost('admin/build/translate/import', array(
'langcode' => 'fr',
'files[file]' => $name,
), t('Import'));
unlink($name);
// The importation should have created 1 string and rejected 2.
$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' => 1, '%update' => 0, '%delete' => 0)), t('The translation file was successfully imported.'));
$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.'));
}
/**
......@@ -189,6 +264,31 @@ msgstr "samedi"
msgid "Sunday"
msgstr "dimanche"
EOF;
}
/**
* Helper function that returns a proper .po file.
*/
function getBadPoFile() {
return <<< EOF
msgid ""
msgstr ""
"Project-Id-Version: Drupal 6\\n"
"MIME-Version: 1.0\\n"
"Content-Type: text/plain; charset=UTF-8\\n"
"Content-Transfer-Encoding: 8bit\\n"
"Plural-Forms: nplurals=2; plural=(n > 1);\\n"
msgid "Save configuration"
msgstr "Enregistrer la configuration"
msgid "edit"
msgstr "modifier<img SRC="javascript:alert(\'xss\');">"
msgid "delete"
msgstr "supprimer<script>alert('xss');</script>"
EOF;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment