Closed Bug 405922 Opened 12 years ago Closed 11 years ago

sanitize data injected into places views from dynamic containers

Categories

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

defect

Tracking

()

RESOLVED INVALID

People

(Reporter: dietrich, Unassigned)

References

Details

(Whiteboard: [sg:moderate] assumes broken addon installed)

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.
Blocks: 375898
Since this is a dependency of bug 375898, marking P2 so it stays on our radar for Fx3 triage/tracking.
Flags: blocking-firefox3?
Priority: -- → P2
Target Milestone: --- → Firefox 3 beta4
"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
Whiteboard: [sg:moderate]
This can only happen via addons.
Whiteboard: [sg:moderate] → [sg:moderate] assumes broken addon installed
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: Firefox 3 beta4 → Firefox 3
Assignee: nobody → dietrich
Assignee: dietrich → ondrej
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.
Ondrej, that's just a tracking bug, will open it after discussion with dveditz tomorrow, but there's no useful data there...
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.
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.
Flags: blocking-firefox3+ → blocking-firefox3?
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.
Flags: blocking-firefox3? → blocking-firefox3-
I guess this could be marked as invalid?
Assignee: ondrej → nobody
Target Milestone: Firefox 3 → ---
Blocks: 453530
No longer blocks: 453530
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?
(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.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.