Confirmed users, Bureaucrats and Sysops emeriti
2,088
edits
Places:SecurityReview: Difference between revisions
→Notes
No edit summary |
(→Notes) |
||
| Line 166: | Line 166: | ||
= Notes = | = Notes = | ||
* SQL Injection | * SQL Injection ({{bug|405920}}) | ||
** TODO: enumerate and document all sources of data added into places.sqlite | ** TODO: enumerate and document all sources of data added into places.sqlite | ||
** TODO: formalized audit of code path of input data, confirm it's using parameter binding instead of executing raw SQL. | ** TODO: formalized audit of code path of input data, confirm it's using parameter binding instead of executing raw SQL. | ||
* Remote Containers | * Remote Containers | ||
** External data shown inside places views | ** External data shown inside places views | ||
** Sanitization of data? Same as input data from user/web? | ** Sanitization of data? Same as input data from user/web? ({{bug|405922}}) | ||
** No visual identification that they're different from local folders | ** No visual identification that they're different from local folders ({{bug|405921}}) | ||
* Assuming Fx is only writer to places.sqlite | * Assuming Fx is only writer to places.sqlite ({{bug|405923}}) | ||
** Extensions can write to it | ** Extensions can write to it | ||
** External apps | ** External apps | ||
** Any solution to this should be application wide, not just places | ** Any solution to this should be application wide, not just places | ||
* Place URIs in content? moz-anno? | * Place URIs in content? moz-anno? ({{bug|405924}}) | ||
** TODO: write mochitest test for this | ** TODO: write mochitest test for this | ||
** TODO: confirm moz-anno blacklisted, LXR link is rotten: http://mxr.mozilla.org/seamonkey/search?string=moz-anno | ** TODO: confirm moz-anno blacklisted, LXR link is rotten: http://mxr.mozilla.org/seamonkey/search?string=moz-anno | ||
* Increased risk from having more history | * Increased risk from having more history | ||
** TODO: document the changes, the toggles, and the elevated risk. | ** TODO: document the changes, the toggles, and the elevated risk. | ||
* Updating to latest SQLite | * Updating to latest SQLite ({{bug|393959}}) | ||
** Need to maintain downrev ourselves if we can't upgrade, even if painful | ** Need to maintain downrev ourselves if we can't upgrade, even if painful | ||
** SQLite is moving fast, not maintaining stable branches | ** SQLite is moving fast, not maintaining stable branches | ||
* Secure deletes | * Secure deletes ({{bug|405925}}) | ||
** SQLite impl, not ours | ** SQLite impl, not ours | ||
** Mil-spec level disk sanitation overkill? | ** Mil-spec level disk sanitation overkill? | ||
** Vlad's async write implementation moots sync write configuration | ** Vlad's async write implementation moots sync write configuration | ||
** Possibly data thought deleted in journal after a crash | ** Possibly data thought deleted in journal after a crash | ||
* Externally manipulated visit counts | * Externally manipulated visit counts ({{bug|405854}}) | ||
** Malicious script bumping up visit counts to push URIs up in autocomplete | ** Malicious script bumping up visit counts to push URIs up in autocomplete | ||
** Window: could only count typed or clicked as visits | ** Window: could only count typed or clicked as visits | ||
| Line 203: | Line 203: | ||
** Should split so hostname is always visible | ** Should split so hostname is always visible | ||
** TODO: make a note on that bug | ** TODO: make a note on that bug | ||
* Javascript/data URIs in the sidebar? | * Javascript/data URIs in the sidebar? ({{bug|405926}}) | ||
** Vulnerable in the past | ** Vulnerable in the past | ||
** Executes in chrome (plugged in 2 and 3) | ** Executes in chrome (plugged in 2 and 3) | ||
| Line 209: | Line 209: | ||
** http://mxr.mozilla.org/seamonkey/ident?i=PU_checkURLSecurity | ** http://mxr.mozilla.org/seamonkey/ident?i=PU_checkURLSecurity | ||
** http://mxr.mozilla.org/seamonkey/source/browser/components/places/content/utils.js#1251 | ** http://mxr.mozilla.org/seamonkey/source/browser/components/places/content/utils.js#1251 | ||
* Feed sanitization | * Feed sanitization ({{bug|405927}}) | ||
** How sanitized now? whitelist based? | ** How sanitized now? whitelist based? | ||
** TODO: confirm whitelist | ** TODO: confirm whitelist | ||
* Feed items that are bookmarklets or data uris | ** Feed items that are bookmarklets or data uris | ||
** TODO: confirm in feeds code | *** TODO: confirm in feeds code | ||
** TODO: if we do drop bad entries, need to document that | *** TODO: if we do drop bad entries, need to document that | ||
* Feed items in autocomplete? | * Feed items in autocomplete? | ||
** mconnor: doctor grp w/ bookmarks in livemarks, and hostname changes regularly, might want livemark items in AC | ** mconnor: doctor grp w/ bookmarks in livemarks, and hostname changes regularly, might want livemark items in AC | ||
** Maybe show livemark item icon instead of star? | ** Maybe show livemark item icon instead of star? | ||
** Maybe have a pref to cover this edge case? | ** Maybe have a pref to cover this edge case? | ||
* JSON in copy/paste | * JSON in copy/paste ({{bug|405929}}) | ||
** If a script could inject JSON data in our format w/ our mime-type into the clipboard, and then get the user to paste while in the Places organizer, could get data into the db | ** If a script could inject JSON data in our format w/ our mime-type into the clipboard, and then get the user to paste while in the Places organizer, could get data into the db | ||
** Relying on crockford's sanitization code | ** Relying on crockford's sanitization code | ||