Closed Bug 289171 Opened 16 years ago Closed 16 years ago
Chrome JS code injection possible using pluginspage attribute
Similar to bug 288556 (not fixed by the patch though) : Code injection into an xul:button's oncommand for manual urls is possible because a oncommand attribute is set. Solution: use event listeners.
Steps to reproduce: - Load testcase - Click on the missing plugin piece - It should say no suitable plugins found, and give a manual install button. - Clicking the manual install button will open a new window and an dialog saying "Code Injection!". When fixed, the dialog should now show (new window is fine).
Status: NEW → ASSIGNED
Correction to above: When fixed, the dialog should _NOT_ show (new window is fine).
Doron: I can't reproduce this with your testcase using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050403 Firefox/1.0.3 . Do I need any special prefs set to see this problem?
The patch to 288556 made this a bit harder to exploit after all, new testcase attached.
Attachment #179733 - Attachment is obsolete: true
Flags: blocking-aviary1.0.3? → blocking-aviary1.0.3+
Fixed the theoretical issue of injection via the link to update for missing plugins. In reality, the server chokes on the malformed url though :)
Comment on attachment 179748 [details] [diff] [review] fix another theoretical issue sorry, missed a file.
Comment on attachment 179737 [details] [diff] [review] patch sr=jst
Comment on attachment 179737 [details] [diff] [review] patch r=dbaron too, although worth noting that this currently leaks (bug 241518). We can live with that, though.
Attachment #179737 - Flags: review?(dbaron) → review+
I could remove the listeners onunload if needed.
Comment on attachment 179756 [details] [diff] [review] fix theoretical issue and remove onclick attribute sr=jst
Attachment #179756 - Flags: superreview?(jst) → superreview+
You (In reply to comment #12) > I could remove the listeners onunload if needed. You actually can't, since you don't have the listener object anymore. But don't worry about it -- we have this leak all over, and plugin install is basically a one-time thing.
Comment on attachment 179756 [details] [diff] [review] fix theoretical issue and remove onclick attribute Please don't forget to land this on the trunk.
fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.