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

RESOLVED FIXED in mozilla15

Status

()

Core
Internationalization
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: hsivonen, Assigned: emk)

Tracking

10 Branch
mozilla15
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 2

6 years ago
(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
(Assignee)

Comment 3

6 years ago
Is it observable from Web pages?
(Reporter)

Comment 4

6 years ago
(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
(Assignee)

Comment 5

5 years ago
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.
Assignee: smontagu → VYV03354
Status: NEW → ASSIGNED
Attachment #619268 - Flags: review?(smontagu)
(Assignee)

Comment 6

5 years ago
Created attachment 619269 [details] [diff] [review]
comm-central change
Attachment #619269 - Flags: review?(dbienvenu)

Comment 7

5 years ago
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+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ead76417212
https://hg.mozilla.org/comm-central/rev/c322601b7f61
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15
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]
(Assignee)

Comment 10

5 years ago
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]
(Assignee)

Comment 11

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [leave open - add checkin-needed after landing on m-c]
https://hg.mozilla.org/mozilla-central/rev/8ead76417212
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.

Updated

5 years ago
Depends on: 763617
You need to log in before you can comment on or make changes to this bug.