Closed
Bug 127702
Opened 23 years ago
Closed 22 years ago
URLs in mail messages can issue IMAP commands
Categories
(MailNews Core :: Security, defect)
MailNews Core
Security
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.0
People
(Reporter: security-bugs, Assigned: mscott)
Details
Attachments
(1 file)
(deleted),
patch
|
cavin
:
review+
Bienvenu
:
superreview+
chofmann
:
approval+
|
Details | Diff | Splinter Review |
Mozilla allows opening imap:// urls by javascript or by just
clicking on them.
Which is more, imap protocol commands may be embeded in the urls, which may
lead to modification of user's folders and probably more.
For example the following link in HTML message:
------------------------------
<a href="imap://USERNAME@SERVER:143/create%3E/guninski%3E%3E">CREATE</a>
[USERNAME and SERVER must be changed]
------------------------------
Creates the imap folder "guninski" if it is clicked (it is possible to get
an error message but the folder is still created).
It is also possible javascript to open imap messages. The same origin
policy prevents them from being read but it is still bad remote content to
open local email.
------------ supplement --------------------
No user interaction is required.
The following in HTML email:
---------------------
<iframe src="imap://USER@HOST:143/create%3E/guninski%3E%3E" ></iframe>
---------------------
Creates the folder GUNINSKI if the message is previewed.
More problems are possible.
For example the following:
----------------------------
<a href="imap://USER@HOST:143/onlineCopy%3EUID%3E/Trash%3E2%3E/INBOX">COPY</a>
----------------------------
Copies on message from Trash to INBOX.
Almost sure deleting messages and even folders is possible.
The good news is this does not work from the web.
I suggest disabling imap: URLs from email, the same way that they are not
accessible from web pages.
Reporter | ||
Updated•23 years ago
|
Severity: normal → critical
Status: NEW → ASSIGNED
Keywords: nsbeta1
Target Milestone: --- → mozilla1.0
Updated•23 years ago
|
Assignee | ||
Comment 1•23 years ago
|
||
taking from Mitch. I'll look into fixing this.
Assignee: mstoltz → mscott
Status: ASSIGNED → NEW
Reporter | ||
Comment 2•23 years ago
|
||
Scott, any progress on this?
Comment 3•23 years ago
|
||
Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.) Thanks!
Comment 4•22 years ago
|
||
It's my understanding that this is one of two security issues that we need fixed
for 1.0 (and any other products based on the branch which are exposed to this
vulnerability). We're shipping RC1 today so we missed that train. Is this gonna
make the next one?
Assignee | ||
Comment 5•22 years ago
|
||
Here's what I'm thinking about doing to solve this problem.
1) Add a restricted capabilities bit to nsIImapUrl.idl
this restricted bit would be set to true every time we create an imap url.
2) Every time our imap service attempts to create a url to perform an action it
clears the restricted bit on the url it creates. Urls generated from the imap
service should not be restricted
Any url that comes from the user clicking on a url or from JS code trying to
load an imap url will end up with a nsIImapUri object which has the restricted
bit set.
3) When we go to run the url, we only run urls with the restricted bit set if
they represent the following actions:
fetching a part
displaying a message
what others?
cc'ing bienvenu to get his input on my solution and to see if he has imap
actions he'd like to add to the restricted list. (Does this effect shared
folders at all? I don't think so)
Status: NEW → ASSIGNED
Comment 6•22 years ago
|
||
we need links to folders to work, i.e., clicking on an imap url to an imap
folder should load that folder.
I was surprised how many imap urls get run without going through nsImapService -
things like view | source, quoting a message, and others I can't remember, so
you need to be very careful. I already have a flag for urls not created by the
imap service; it's a boolean called m_externalLinkUrl and I think you should be
able to use it.
The other possibility is distinguish between imap uri's like links to messages
and folders, and *internal imap commands* constructed by nsImapService, things
like "create folder", "delete folder" etc, which really should be only allowed
from internal urls. So I guess what I'm suggesting is looking at the imap action
in conjunction with the m_externalLinkUrl and only allow external link urls to
fetch messages/parts and load folders.
Assignee | ||
Comment 7•22 years ago
|
||
Wow thanks David. How convient that you've already added the flag:
m_externalLinkUrl. That will make this fix trivial in code (but not in risk of
course). Thanks for the tip.
Assignee | ||
Comment 8•22 years ago
|
||
I think I'm going to change this patch, but here's the first pass.
The following code lives in nsImapMockChannel::AsyncOpen (which is the only way
to open an imap url). If the url is an external link, then we only allow the
following actions:
1) nsIImapUrl::nsImapSelectFolder
2) nsIImapUrl::nsImapMsgFetch
3) nsIImapUrl::nsImapOpenMimePart
However I think I'm going to change this patch and add a method to nsIURI to
the effect of validUrl() which does this check and returns a boolean if we are
allowed to open the url. In the future, other consumers may be able to leverage
this security check instead of just coding it directly into AsyncOpen. Then
again AsyncOpen really is the only way to open the url so that extra step may
be unnecessary.
Comment 9•22 years ago
|
||
The initial fix patch was attached three weeks ago -- what's new? We need this
fixed ASAP (today, ideally) for 1.0.
/be
Blocks: 143200
Assignee | ||
Comment 10•22 years ago
|
||
I doubt you'll want to take this for moz 1.0. I need to get this into the trunk
and it needs baking time. There are too many ways in which it could break imap
(replying to messages, quoting replied messages, saving / opening attachments,
etc).
My gut would say pass on this for 1.0.
Comment 11•22 years ago
|
||
mscott, 1.0 shipping with a security hole like this means that we have to
identify the hole and summarize it to anyone thinking of using 1.0. Maybe you
don't care, so long as this is fixed for Netscape rtm (and presumably 1.0.1).
Either we fix, or we disclose enough information for 1.0 users to make informed
decisions, though. Does this alter your thinking?
/be
Assignee | ||
Comment 12•22 years ago
|
||
I told Dawn a fib just now. I didn't think you could really delete a message
through this kind of an exploit, just cause the user to select one of their
folders or select a message. But I was just able to force a url which would
delete a message and sure enough we ran it when I clicked on it(even though the
messsage key didn't exist in my case). With this patch we don't even try to run
the url.
That test does change my thinking on this bug. I'll get r and sr today for it.
But we still have to recognize some risk for 1.0 by taking it today without some
trunk baking time.
Assignee | ||
Comment 13•22 years ago
|
||
cc'ing Cavin since I'm going to have him review this patch and you have to be
cc'ed in order to see it.
Comment 14•22 years ago
|
||
Comment on attachment 81550 [details] [diff] [review]
initial fix
r=cavin.
Attachment #81550 -
Flags: review+
Comment 15•22 years ago
|
||
Comment on attachment 81550 [details] [diff] [review]
initial fix
sr=bienvenu - let's try this out asap. This might require some tweaking, but
it's probably better to start out conservatively in the list of things we
allow.
Attachment #81550 -
Flags: superreview+
Assignee | ||
Comment 16•22 years ago
|
||
fixed on the trunk. Marking fixed. I'll add the fixed1.0.0 field when I get
approval for the branch.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 17•22 years ago
|
||
Comment on attachment 81550 [details] [diff] [review]
initial fix
lets go ahead and get this on the branch as soon as possible so it can get
extra testing. a=chofmann,blizzard
Attachment #81550 -
Flags: approval+
Assignee | ||
Comment 18•22 years ago
|
||
thanks for the approval Chris. adding fixed 1.0.0 keyword
Keywords: fixed1.0.0
Comment 19•22 years ago
|
||
Has this been checked into the branch? I assume so given the fixed1.0.0, but
the comment didn't really say.
Comment 20•22 years ago
|
||
bonsai shows that this was checked in to branch on 05/20/2002 22:31 by mscott
Comment 22•22 years ago
|
||
Used 06-03-08-trunk build
If above reporter's original URL is correct,
"imap://USERNAME@SERVER:143/create%3E/foldername%3E%3E"
Then I am wondering to know why am I getting the following dialogs for this fix?
"Would you like to subscribe to Shared Folders/User/3qatest01/karenfolder?"
After I select OK, I got "The current command did not succeed. The
mail server responded: Mailboxes does not exist."
It seems this fix is trying to call bug 112105 fix....
Is this expected result?
Assignee | ||
Comment 23•22 years ago
|
||
Example URL for QA to use for testing...replace username with your username on
the specified mail server. Try inserting this url into a mail message, send it
to yourself then click on the url. For bonus points use a real message key from
your inbox by looking at an imap log and replacing the numbers in this URL with
a real message key in your account.
imap://username@dredd.mcom.com:143/onlinemove%3EUID%3E/INBOX%3E190038%3E/Trash"
imap://username@dredd.mcom.com:143/deletemsg%3EUID%3E/INBOX%3E190038%3E/
Comment 24•22 years ago
|
||
Verified fix on all the platforms on current branch builds:
Windows 07-11-08-1.0
Linux 07-11-07-1.0
Mac 07-11-05-1.0
*Before fix:
-For UID 4100 (for move):
1364[4add468]: nsmail-1:S-INBOX:SendData: 19 uid copy 4100 "Trash"
-For UID 4104 (for delete):
1364[4a9add8]: nsmail-1:S-INBOX:SendData: 11 uid store 4104 +FLAGS (\Deleted)
-For UID 4102 (for copy)
1364[4a9add8]: nsmail-1:S-INBOX:SendData: 14 uid copy 4102 "Trash"
1364[4a9add8]: nsmail-1:S-INBOX:CreateNewLineFromSocket: 14 OK [COPYUID
964732484 4102 3106] Completed
After this bug fix, I don't see move, delete and copy implementation from Inbox
to Trash folder on the UI and the IMAP log.
The only rest issue for create command(see above Comment #22)has been logged on
Bug 157037.
Marking as verified and changing keywords from "fixed1.0.0" to "verified1.0.1"
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.0 → verified1.0.1
Reporter | ||
Updated•22 years ago
|
Group: security?
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•