Last Comment Bug 710693 - x-imap4-modified-utf7 is exposed as a supported encoding name
: x-imap4-modified-utf7 is exposed as a supported encoding name
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: 10 Branch
: All All
: -- normal (vote)
: mozilla15
Assigned To: Masatoshi Kimura [:emk]
:
Mentors:
Depends on: 763617
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-14 06:48 PST by Henri Sivonen (:hsivonen)
Modified: 2012-06-13 09:28 PDT (History)
10 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Move internal encoding check into nsCharsetAlias (20.49 KB, patch)
2012-04-28 02:49 PDT, Masatoshi Kimura [:emk]
smontagu: review+
Details | Diff | Review
comm-central change (1.12 KB, patch)
2012-04-28 02:51 PDT, Masatoshi Kimura [:emk]
mozilla: review+
Details | Diff | Review

Description Henri Sivonen (:hsivonen) 2011-12-14 06:48:14 PST
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.
Comment 1 Simon Montagu :smontagu 2011-12-14 11:14:50 PST
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.
Comment 2 Henri Sivonen (:hsivonen) 2011-12-16 01:08:57 PST
(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.)
Comment 3 Masatoshi Kimura [:emk] 2011-12-19 15:03:02 PST
Is it observable from Web pages?
Comment 4 Henri Sivonen (:hsivonen) 2011-12-20 01:01:04 PST
(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
Comment 5 Masatoshi Kimura [:emk] 2012-04-28 02:49:52 PDT
Created attachment 619268 [details] [diff] [review]
Move internal encoding check into nsCharsetAlias

- 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.
Comment 6 Masatoshi Kimura [:emk] 2012-04-28 02:51:29 PDT
Created attachment 619269 [details] [diff] [review]
comm-central change
Comment 7 David :Bienvenu 2012-05-02 14:04:02 PDT
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).
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-05-24 18:32:14 PDT
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
Comment 10 Masatoshi Kimura [:emk] 2012-05-24 18:56:27 PDT
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!
Comment 11 Masatoshi Kimura [:emk] 2012-05-24 22:24:59 PDT
And after merge, c-c build will crash without the c-c patch.
Comment 12 Mike Conley (:mconley) - (needinfo me!) 2012-05-25 08:13:41 PDT
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.
Comment 13 Mike Conley (:mconley) - (needinfo me!) 2012-05-25 08:17:44 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/a404d51fbb91
Comment 14 Ed Morley [:emorley] 2012-05-25 08:26:21 PDT
https://hg.mozilla.org/mozilla-central/rev/8ead76417212
Comment 15 Mike Conley (:mconley) - (needinfo me!) 2012-05-25 08:29:03 PDT
Whoops - sorry for the premature resolve of the bug before m-c got in. Muscle memory.
Comment 16 neil@parkwaycc.co.uk 2012-06-13 06:37:52 PDT
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.)
Comment 17 neil@parkwaycc.co.uk 2012-06-13 06:42:17 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.