| Index: base/i18n/file_util_icu.cc
|
| diff --git a/base/i18n/file_util_icu.cc b/base/i18n/file_util_icu.cc
|
| index b5c0b9d8f5f9f661c5c7965bdb6fbb1e39639ed5..f57bb396a9162c0a9b4b3e5ae124d3add51afa7e 100644
|
| --- a/base/i18n/file_util_icu.cc
|
| +++ b/base/i18n/file_util_icu.cc
|
| @@ -30,12 +30,19 @@ class IllegalCharacters {
|
| return Singleton<IllegalCharacters>::get();
|
| }
|
|
|
| - bool contains(UChar32 ucs4) {
|
| - return !!set->contains(ucs4);
|
| + bool DisallowedEverywhere(UChar32 ucs4) {
|
| + return !!illegal_anywhere_->contains(ucs4);
|
| }
|
|
|
| - bool containsNone(const string16 &s) {
|
| - return !!set->containsNone(icu::UnicodeString(s.c_str(), s.size()));
|
| + bool DisallowedLeadingOrTrailing(UChar32 ucs4) {
|
| + return !!illegal_at_ends_->contains(ucs4);
|
| + }
|
| +
|
| + bool IsAllowedName(const string16& s) {
|
| + return s.empty() || (!!illegal_anywhere_->containsNone(
|
| + icu::UnicodeString(s.c_str(), s.size())) &&
|
| + !illegal_at_ends_->contains(*s.begin()) &&
|
| + !illegal_at_ends_->contains(*s.rbegin()));
|
| }
|
|
|
| private:
|
| @@ -45,60 +52,61 @@ class IllegalCharacters {
|
| IllegalCharacters();
|
| ~IllegalCharacters() { }
|
|
|
| - scoped_ptr<icu::UnicodeSet> set;
|
| + // set of characters considered invalid anywhere inside a filename.
|
| + scoped_ptr<icu::UnicodeSet> illegal_anywhere_;
|
| +
|
| + // set of characters considered invalid at either end of a filename.
|
| + scoped_ptr<icu::UnicodeSet> illegal_at_ends_;
|
|
|
| DISALLOW_COPY_AND_ASSIGN(IllegalCharacters);
|
| };
|
|
|
| IllegalCharacters::IllegalCharacters() {
|
| - UErrorCode status = U_ZERO_ERROR;
|
| - // Control characters, formatting characters, non-characters, and
|
| - // some printable ASCII characters regarded as dangerous ('"*/:<>?\\').
|
| + UErrorCode everywhere_status = U_ZERO_ERROR;
|
| + UErrorCode ends_status = U_ZERO_ERROR;
|
| + // Control characters, formatting characters, non-characters, path separators,
|
| + // and some printable ASCII characters regarded as dangerous ('"*/:<>?\\').
|
| // See http://blogs.msdn.com/michkap/archive/2006/11/03/941420.aspx
|
| // and http://msdn2.microsoft.com/en-us/library/Aa365247.aspx
|
| - // TODO(jungshik): Revisit the set. ZWJ and ZWNJ are excluded because they
|
| - // are legitimate in Arabic and some S/SE Asian scripts. However, when used
|
| - // elsewhere, they can be confusing/problematic.
|
| - // Also, consider wrapping the set with our Singleton class to create and
|
| - // freeze it only once. Note that there's a trade-off between memory and
|
| - // speed.
|
| -#if defined(WCHAR_T_IS_UTF16)
|
| - set.reset(new icu::UnicodeSet(icu::UnicodeString(
|
| - L"[[\"*/:<>?\\\\|][:Cc:][:Cf:] - [\u200c\u200d]]"), status));
|
| -#else
|
| - set.reset(new icu::UnicodeSet(UNICODE_STRING_SIMPLE(
|
| - "[[\"*/:<>?\\\\|][:Cc:][:Cf:] - [\\u200c\\u200d]]").unescape(),
|
| - status));
|
| -#endif
|
| - DCHECK(U_SUCCESS(status));
|
| + // Note that code points in the "Other, Format" (Cf) category are ignored on
|
| + // HFS+ despite the ZERO_WIDTH_JOINER and ZERO_WIDTH_NON-JOINER being
|
| + // legitimate in Arabic and some S/SE Asian scripts. In addition tilde (~) is
|
| + // also excluded due to the possibility of interacting poorly with short
|
| + // filenames on VFAT. (Related to CVE-2014-9390)
|
| + illegal_anywhere_.reset(new icu::UnicodeSet(
|
| + UNICODE_STRING_SIMPLE("[[\"~*/:<>?\\\\|][:Cc:][:Cf:]]"),
|
| + everywhere_status));
|
| + illegal_at_ends_.reset(new icu::UnicodeSet(
|
| + UNICODE_STRING_SIMPLE("[[:WSpace:][.]]"), ends_status));
|
| + DCHECK(U_SUCCESS(everywhere_status));
|
| + DCHECK(U_SUCCESS(ends_status));
|
| +
|
| // Add non-characters. If this becomes a performance bottleneck by
|
| // any chance, do not add these to |set| and change IsFilenameLegal()
|
| // to check |ucs4 & 0xFFFEu == 0xFFFEu|, in addiition to calling
|
| - // containsNone().
|
| - set->add(0xFDD0, 0xFDEF);
|
| + // IsAllowedName().
|
| + illegal_anywhere_->add(0xFDD0, 0xFDEF);
|
| for (int i = 0; i <= 0x10; ++i) {
|
| int plane_base = 0x10000 * i;
|
| - set->add(plane_base + 0xFFFE, plane_base + 0xFFFF);
|
| + illegal_anywhere_->add(plane_base + 0xFFFE, plane_base + 0xFFFF);
|
| }
|
| - set->freeze();
|
| + illegal_anywhere_->freeze();
|
| + illegal_at_ends_->freeze();
|
| }
|
|
|
| } // namespace
|
|
|
| bool IsFilenameLegal(const string16& file_name) {
|
| - return IllegalCharacters::GetInstance()->containsNone(file_name);
|
| + return IllegalCharacters::GetInstance()->IsAllowedName(file_name);
|
| }
|
|
|
| void ReplaceIllegalCharactersInPath(FilePath::StringType* file_name,
|
| char replace_char) {
|
| - DCHECK(file_name);
|
| -
|
| - DCHECK(!(IllegalCharacters::GetInstance()->contains(replace_char)));
|
| + IllegalCharacters* illegal = IllegalCharacters::GetInstance();
|
|
|
| - // Remove leading and trailing whitespace.
|
| - TrimWhitespace(*file_name, TRIM_ALL, file_name);
|
| + DCHECK(!(illegal->DisallowedEverywhere(replace_char)));
|
| + DCHECK(!(illegal->DisallowedLeadingOrTrailing(replace_char)));
|
|
|
| - IllegalCharacters* illegal = IllegalCharacters::GetInstance();
|
| int cursor = 0; // The ICU macros expect an int.
|
| while (cursor < static_cast<int>(file_name->size())) {
|
| int char_begin = cursor;
|
| @@ -122,7 +130,9 @@ void ReplaceIllegalCharactersInPath(FilePath::StringType* file_name,
|
| NOTREACHED();
|
| #endif
|
|
|
| - if (illegal->contains(code_point)) {
|
| + if (illegal->DisallowedEverywhere(code_point) ||
|
| + ((char_begin == 0 || cursor == static_cast<int>(file_name->length())) &&
|
| + illegal->DisallowedLeadingOrTrailing(code_point))) {
|
| file_name->replace(char_begin, cursor - char_begin, 1, replace_char);
|
| // We just made the potentially multi-byte/word char into one that only
|
| // takes one byte/word, so need to adjust the cursor to point to the next
|
|
|