Closed Bug 710693 Opened 14 years ago Closed 14 years ago

x-imap4-modified-utf7 is exposed as a supported encoding name

Categories

(Core :: Internationalization, defect)

Product:
Core
Component:
Internationalization
Version:
10 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla15

People

(Reporter: hsivonen, Assigned: emk)

References

Details

Attachments

(2 files)

Move internal encoding check into nsCharsetAlias
20.49 KB, patch
smontagu
: review+
Details | Diff | Splinter Review
comm-central change
1.12 KB, patch
Bienvenu
: review+
Details | Diff | Splinter Review
The encoding x-imap4-modified-utf7 seems to be available for Web content to use. If it has the same XSS characteristics as UTF-7, exposing it to content is bad. In any case, it doesn't make sense to allow that kind of special encoding to be used outside the context for which it was designed, i.e. the IMAP protocol itself.
Do you have a testcase for this? I tried to create a page with <meta charset="x-imap4-modified-utf7">, but it wasn't recognized.
(In reply to Simon Montagu from comment #1) > Do you have a testcase for this? I tried to create a page with <meta > charset="x-imap4-modified-utf7">, but it wasn't recognized. The HTML parser explicitly blacklists this encoding for <meta>. However, http://hsivonen.iki.fi/p/imap/imap.php shows (if stepping through it in a debugger) that the decoder instantiation is blocked. So the problem here is that the charset alias API that's used for determining if an encoding is supported behaves as if encodings annotated as ".isXSSVulnerable" were supported. I think the charset alias API should behave as if ".isXSSVulnerable" encodings didn't exist so that we get the APIs back in sync so that any "preferred" encoding name that the alias API returns is accepted by the "Raw" variants of the decoder factory. (I'm not allowed remove the security bug status from this bug.)
Summary: x-imap4-modified-utf7 is exposed to Web content → x-imap4-modified-utf7 is exposed as a supported encoding name
Group: core-security
Is it observable from Web pages?
(In reply to Masatoshi Kimura [:emk] from comment #3) > Is it observable from Web pages? Yes: Compare http://hsivonen.iki.fi/test/moz/imap/baseline-xml.php and http://hsivonen.iki.fi/test/moz/imap/imap-xml.php
- Removed "RawInternal" variant from nsCharsetConverterManager. "Raw" variants do no longer check internal encodings. Internal encodings test will be performed anyway on resolving the alias. - Make some nsCharsetConverterManager methods static so that nsChasetAlias can call them.
Assignee: smontagu → VYV03354
Status: NEW → ASSIGNED
Attachment #619268 - Flags: review?(smontagu)
Attachment #619269 - Flags: review?(dbienvenu)
Comment on attachment 619269 [details] [diff] [review] comm-central change imap mod utf7 still seems to be working (I assume that's what you wanted me to check).
Attachment #619269 - Flags: review?(dbienvenu) → review+
Attachment #619268 - Flags: review?(smontagu) → review+
Keywords: checkin-needed
I had to backout the comm-central patch due to xpcshell orange. https://hg.mozilla.org/comm-central/rev/fbfae44b4c5a https://tbpl.mozilla.org/php/getParsedLog.php?id=12044708&tree=Thunderbird-Trunk TEST-INFO | /Users/cltbld/talos-slave/test/build/xpcshell/tests/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js | running test ... TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js | test failed (with xpcshell return code: 0), see following log: >>>>>>> ### XPCOM_MEM_LEAK_LOG defined -- logging leaks to /var/folders/Hs/HsDn6a9SG8idoIya6p9mtE+++TI/-Tmp-/tmpiJJ_Fa/runxpcshelltests_leaks.log 2012-05-24 16:32:01 gloda.NS INFO Logging Initialized 2012-05-24 16:32:01 gloda.datastore DEBUG Beginning datastore initialization. 2012-05-24 16:32:01 gloda.datastore DEBUG Creating database because it doesn't exist. 2012-05-24 16:32:01 gloda.datastore INFO Creating table: folderLocations 2012-05-24 16:32:01 gloda.datastore INFO Creating table: conversations 2012-05-24 16:32:01 gloda.datastore INFO Creating fulltext table: CREATE VIRTUAL TABLE conversationsText USING fts3(tokenize mozporter, subject TEXT) 2012-05-24 16:32:01 gloda.datastore INFO Creating table: messages 2012-05-24 16:32:01 gloda.datastore INFO Creating fulltext table: CREATE VIRTUAL TABLE messagesText USING fts3(tokenize mozporter, body TEXT, subject TEXT, attachmentNames TEXT, author TEXT, recipients TEXT) 2012-05-24 16:32:01 gloda.datastore INFO Creating table: attributeDefinitions 2012-05-24 16:32:01 gloda.datastore INFO Creating table: messageAttributes 2012-05-24 16:32:01 gloda.datastore INFO Creating table: contacts 2012-05-24 16:32:01 gloda.datastore INFO Creating table: contactAttributes 2012-05-24 16:32:01 gloda.datastore INFO Creating table: identities 2012-05-24 16:32:01 gloda.datastore DEBUG Initializing folder mappings. 2012-05-24 16:32:01 gloda.datastore DEBUG Populating managed id counters. 2012-05-24 16:32:01 gloda.datastore DEBUG Completed datastore initialization. 2012-05-24 16:32:01 gloda.NS INFO Defining noun: bool 2012-05-24 16:32:01 gloda.NS INFO Defining noun: number 2012-05-24 16:32:01 gloda.NS INFO Defining noun: string 2012-05-24 16:32:01 gloda.NS INFO Defining noun: date 2012-05-24 16:32:01 gloda.NS INFO Defining noun: fulltext 2012-05-24 16:32:01 gloda.NS INFO Defining noun: folder 2012-05-24 16:32:01 gloda.NS INFO Defining noun: account 2012-05-24 16:32:01 gloda.NS INFO Defining noun: conversation 2012-05-24 16:32:01 gloda.NS INFO Defining noun: message 2012-05-24 16:32:01 gloda.NS INFO Defining noun: contact 2012-05-24 16:32:01 gloda.NS INFO Defining noun: identity 2012-05-24 16:32:01 gloda.NS INFO Defining noun: attachment-infos 2012-05-24 16:32:01 gloda.NS INFO Defining noun: parameterized-identity 2012-05-24 16:32:01 gloda.datastore INFO loading all attribute defs 2012-05-24 16:32:01 gloda.datastore INFO done loading all attribute defs WARNING: NS_ENSURE_TRUE(!(accountList.IsEmpty())) failed: file ../../../../mailnews/base/src/nsMsgAccountManager.cpp, line 1343 2012-05-24 16:32:01 gloda.everybody INFO ... loading resource:///modules/gloda/fundattr.js 2012-05-24 16:32:01 gloda.NS INFO Defining noun: mime-type 2012-05-24 16:32:01 gloda.datastore INFO Creating table: ext_mimeTypes 2012-05-24 16:32:01 gloda.noun.mimetype DEBUG loading MIME types 2012-05-24 16:32:01 gloda.datastore DEBUG QUERY FROM QUERY: SELECT * FROM ext_mimeTypes ARGS: Skipping 5121 lines... 2012-05-24 16:32:21 gloda.index_msg DEBUG * Got Mime Message (20 bytes): Lame Meeting Today 1 Body: text/plain (20 bytes) 2012-05-24 16:32:21 gloda.datastore DEBUG QUERY FROM QUERY: SELECT * FROM messages LEFT JOIN messagesText ON messages.id = messagesText.rowid WHERE (id IN (SELECT id FROM messages WHERE (headerMessageID IN ('59@made.up')))) ARGS: 2012-05-24 16:32:21 gloda.index_msg.mbm DEBUG getting results... 2012-05-24 16:32:21 gloda.index_msg.mbm DEBUG query completed, notifying... 2012-05-24 16:32:21 gloda.index_msg DEBUG ancestors raw: 2012-05-24 16:32:21 gloda.index_msg DEBUG ref len: 1 anc len: 1 2012-05-24 16:32:21 gloda.index_msg DEBUG references: 0 = 59@made.up 2012-05-24 16:32:21 gloda.index_msg DEBUG ancestors: 0 = 2012-05-24 16:32:21 gloda.index_msg DEBUG 0 candidate messages 2012-05-24 16:32:21 gloda.NS INFO ** grokNounItem: message 2012-05-24 16:32:21 gloda.NS INFO * provider: gloda.fundattr 2012-05-24 16:32:21 gloda.datastore DEBUG QUERY FROM QUERY: SELECT * FROM identities WHERE (id IN (SELECT id FROM identities WHERE (kind IN ('email')) AND (value IN ('olof@rolinski.nul','pete@stanley.nul')))) ARGS: 2012-05-24 16:32:21 gloda.NS DEBUG creating contact for 'Olof Rolinski' (olof@rolinski.nul) 2012-05-24 16:32:21 gloda.NS DEBUG creating contact for 'Pete Stanley' (pete@stanley.nul) 2012-05-24 16:32:21 gloda.NS INFO * provider: gloda.explattr 2012-05-24 16:32:21 gloda.NS INFO * provider: glodaTestHelper:fakeProvider 2012-05-24 16:32:21 gloda.NS INFO * optimizer: gloda.fundattr 2012-05-24 16:32:21 gloda.NS INFO ** done with providers. 2012-05-24 16:32:21 gloda.NS DEBUG json text: {"43":87,"44":[88],"45":[],"46":[],"50":false,"51":[],"58":false,"59":false,"60":false,"61":false,"57":[],"52":[87,88],"53":[88]} 2012-05-24 16:32:21 gloda.NS DEBUG inserting item 2012-05-24 16:32:21 gloda.NS DEBUG adjusting attributes, add: 58,0,1,57,52,87,52,88 rem: 2012-05-24 16:32:21 gloda.NS DEBUG done grokking. 2012-05-24 16:32:21 gloda.indexer INFO --- Done indexing, disabling timer renewal. 2012-05-24 16:32:21 gloda.test DEBUG ((( Index listener notified! aStatus = 0 waiting: true 2012-05-24 16:32:21 gloda.test DEBUG kicking driver... TEST-INFO | (xpcshell/head.js) | test 2 pending TEST-INFO | (xpcshell/head.js) | test 2 finished WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file ../../../../mailnews/base/util/nsMsgI18N.cpp, line 167 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file ../../../../mailnews/base/util/nsMsgI18N.cpp, line 167 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file ../../../../mailnews/base/util/nsMsgI18N.cpp, line 167 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file ../../../../mailnews/base/util/nsMsgI18N.cpp, line 167 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file ../../../../mailnews/imap/src/nsImapMailFolder.cpp, line 8656 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file ../../../../mailnews/base/util/nsMsgI18N.cpp, line 167 WARNING: NS_ENSURE_TRUE(!name.IsEmpty()) failed: file ../../../../mailnews/imap/src/nsImapIncomingServer.cpp, line 1330 TEST-UNEXPECTED-FAIL | ../../../../resources/asyncTestUtils.js | Timeout running test, and we want you to have the log. - See following stack: JS frame :: /Users/cltbld/talos-slave/test/build/xpcshell/head.js :: do_throw :: line 440 JS frame :: ../../../../resources/asyncTestUtils.js :: _async_test_runner_timeout :: line 226 JS frame :: /Users/cltbld/talos-slave/test/build/xpcshell/head.js :: <TOP_LEVEL> :: line 121 TEST-INFO | (xpcshell/head.js) | exiting test TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/head.js | exception thrown from do_timeout callback: 2147500036 - See following stack: JS frame :: /Users/cltbld/talos-slave/test/build/xpcshell/head.js :: do_throw :: line 440 JS frame :: /Users/cltbld/talos-slave/test/build/xpcshell/head.js :: <TOP_LEVEL> :: line 123
Whiteboard: [leave open]
The c-c patch depends on the m-c patch. Please wait until the m-c patch has been merged from m-i. I should have mentioned that on the whiteboard. Sorry!
Whiteboard: [leave open] → [leave open - add checkin-needed after landing on m-c]
And after merge, c-c build will crash without the c-c patch.
The patch has merged from m-i to m-c, and I'm seeing the build failure that emk was talking about on c-c. I'm gonna go ahead and try to land this on c-c now.
comm-central: https://hg.mozilla.org/comm-central/rev/a404d51fbb91
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [leave open - add checkin-needed after landing on m-c]
Whoops - sorry for the premature resolve of the bug before m-c got in. Muscle memory.
This exposed bug 763617. Old SeaMonkey code was passing an empty character set to the HTML5 parser, which tried and failed to create a unicode decoder for it. Now previously this always returned NS_ERROR_UCONV_NOCONV, but you now check up front for an empty character set, returning NS_ERROR_NULL_POINTER, which the HTML5 parser doesn't know how to deal with. (Normally if the character set is broken it will fall back to windows-1252.)
More precisely, you propagate the error code from failing to look up the charset alias, rather than ignoring it and failing to create a converter.
Depends on: 763617
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: