405922 - sanitize data injected into places views from dynamic containers

Status: RESOLVED INVALID

(Firefox :: Bookmarks & History, defect, P2)

Description

[Dietrich Ayala (:dietrich)]

Dynamic containers can present data to the user that has not gone through the sanitization done when adding a bookmark, for example. We should sanitize on the fly when displaying this data.

Comment 1

[Brandon Sterne (:bsterne)]

Since this is a dependency of bug 375898, marking P2 so it stays on our radar for Fx3 triage/tracking.

Comment 2

[Daniel Veditz [:dveditz]]

"moderate" because I assume it requires a broken addon to be installed to get unsanitized data into a dynamic container -- do all the stock code paths currently correctly sanitize? If an unmodified FF3 can end up with unsanitized data this could be potentially critical

Comment 3

[Dietrich Ayala (:dietrich)]

This can only happen via addons.

Comment 4

[Ondrej Brablc]

I do not have access to bug 375898 and I do not understand how the data can get to database (addon using mozStorage?) and why it should be limited to dynamic containers only.

Comment 5

[Mike Connor [:mconnor]]

Ondrej, that's just a tracking bug, will open it after discussion with dveditz tomorrow, but there's no useful data there...

Comment 6

[Ondrej Brablc]

The tracking bug and wiki page did not tell me much. Can someone tell me the scenario should I check? Does it really make any sense to sanitize the data if it can only be caused by add-on? If someone replaces my bookmark to my bank with their own URL in places, the data will be properly valid, but I will give them my password, unless I check the address bar.

Comment 7

[Dietrich Ayala (:dietrich)]

The idea here was that somewhere in the process of displaying items from dynamic containers that we should filter out URIs that shouldn't be bookmark-able, eg:

http://mxr.mozilla.org/seamonkey/source/toolkit/components/places/src/nsNavHistory.cpp#2316

However, I'm not sure that it's desirable to do that, given that we may be limiting non-malicious valid use-cases.

Also, any extension could circumvent these filters by modifying the DOM directly, or using the widget apis directly, etc.

Basically, extensions are useful *because* they have the "keys to the kingdom" already. I think the problem this bug addresses is best resolved by extension review.

Resetting blocking flag, as we should not block on this.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

I agree with Dietrich. If the net is "add-ons can do evil things", then we know that already, and it's never really been our policy to keep that from happening.

Comment 9

[Ondrej Brablc]

I guess this could be marked as invalid?

Comment 10

[Jesse Ruderman]

If our API makes it easy to write accidentally insecure addons and hard to write secure addons, that's a problem.  But "addons can do evil things" is not a problem.

Is this a case of an API making it too easy to do insecure things, or a case of an API making it possible to do evil things?

Comment 11

[Dietrich Ayala (:dietrich)]

(In reply to comment #10)
> If our API makes it easy to write accidentally insecure addons and hard to
> write secure addons, that's a problem.  But "addons can do evil things" is not
> a problem.
> 
> Is this a case of an API making it too easy to do insecure things, or a case of
> an API making it possible to do evil things?

The latter. This is not a problem inherent to the dynamic container feature: Any add-on can create any kind of bookmark/folder structure in the user's bookmarks collection using the normal Places APIs. Marking INVALID.

Comment 12

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv