Last Comment Bug 257523 - Text fields give scripts access to the user's clipboard
: Text fields give scripts access to the user's clipboard
[sg:fix] fixed1.7.3
: fixed-aviary1.0, fixed1.7, regression
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: Wladimir Palant
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2004-08-31 06:18 PDT by Wladimir Palant
Modified: 2007-11-17 11:27 PST (History)
12 users (show)
chofmann: blocking1.7.5+
chofmann: blocking‑aviary1.0PR+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Testcase - clipboard read (448 bytes, text/html)
2004-08-31 06:19 PDT, Wladimir Palant
no flags Details
Testcase - clipboard write (463 bytes, text/html)
2004-08-31 06:20 PDT, Wladimir Palant
no flags Details
Proposed patch (1.15 KB, patch)
2004-09-01 04:38 PDT, Wladimir Palant
jst: review+
dveditz: superreview+
chofmann: approval‑aviary+
chofmann: approval1.7.5+
Details | Diff | Splinter Review
Patch that was checked in (2.08 KB, patch)
2004-09-01 17:28 PDT, Johnny Stenback (:jst,
no flags Details | Diff | Splinter Review
Patch for the regression (2.30 KB, patch)
2004-09-29 06:03 PDT, Wladimir Palant
jst: review+
dveditz: superreview+
mozilla: approval‑aviary+
mozilla: approval1.7.5+
Details | Diff | Splinter Review

Description Wladimir Palant 2004-08-31 06:18:04 PDT
Text fields accept script-generated events. You cannot send Ctrl-C - this is a
menu shortcut, so untrusted events will be rejected. But you can send Ctrl-Ins,
which is implemented in the text field itself. This way you can put every text
you want into the user's clipboard. And you can send Shift-Ins to insert
whatever you have in your clipboard into the text field.

I've tested this on Windows but it should work as well at least with KDE, it has
same shortcuts.
Comment 1 Wladimir Palant 2004-08-31 06:19:17 PDT
Created attachment 157492 [details]
Testcase - clipboard read
Comment 2 Wladimir Palant 2004-08-31 06:20:03 PDT
Created attachment 157493 [details]
Testcase - clipboard write
Comment 3 Wladimir Palant 2004-08-31 06:50:19 PDT
These shortcuts are defined in
content/xbl/builtin/(win|unix)/platformHTMLBindings.xml. XBL only rejects
non-trusted events for chrome content but these text fields are not chrome...
Comment 4 Ben Bucksch (:BenB) 2004-08-31 22:04:30 PDT
Severity critical, because I keep copy&pasting passwords, assume others do as well.
Reproduced both read and write on Mozilla 1.7.x branch on Linux. Only works when
I copied using the explicit "CLIPBOARD" (via menu, shortcut etc.), not the
"PRIMARY" clipboard (just select).
Comment 5 Wladimir Palant 2004-09-01 04:38:36 PDT
Created attachment 157608 [details] [diff] [review]
Proposed patch

This prevents commands from being triggered by non-trusted events, even if the
widget itself isn't trusted.
Comment 6 Johnny Stenback (:jst, 2004-09-01 10:11:23 PDT
Comment on attachment 157608 [details] [diff] [review]
Proposed patch

Seems reasonable to me. r=jst
Comment 7 Daniel Veditz [:dveditz] 2004-09-01 11:46:44 PDT
Comment on attachment 157608 [details] [diff] [review]
Proposed patch

>   // See if our event receiver is a content node (and not us).
>   PRBool isXULKey = (mType & NS_HANDLER_TYPE_XUL);
>-  //XUL handlers shouldn't be triggered by non-trusted events.
>-  if (isXULKey) {
>+  //XUL handlers and commands shouldn't be triggered by non-trusted events.
>+  if (isXULKey || (mType & NS_HANDLER_TYPE_XBL_COMMAND)) {

Since there's another (mType & NS_HANDLER_TYPE_XBL_COMMAND) a few lines down
you could define isXBLCommand like isXULKey above for readability. Or not --
your call.

Comment 8 Daniel Veditz [:dveditz] 2004-09-01 11:50:00 PDT
Is there anything that could be legitimately relying on the old behavior? I was
thinking of some sort of QA test harness poking in events, but I guess a chrome
app could create trusted events for that.
Comment 9 chris hofmann 2004-09-01 15:18:39 PDT
Comment on attachment 157608 [details] [diff] [review]
Proposed patch

a=chofmann for the branches
Comment 10 timeless 2004-09-01 15:34:58 PDT
we run as chrome and i actually have keyhandling off. supporting midas is going
to be so much fun.
Comment 11 Johnny Stenback (:jst, 2004-09-01 17:28:03 PDT
Created attachment 157668 [details] [diff] [review]
Patch that was checked in
Comment 12 Johnny Stenback (:jst, 2004-09-01 17:36:05 PDT
Fixed on aviary and 1.7 branches, and on the trunk. Marking FIXED.
Comment 13 Daniel Veditz [:dveditz] 2004-09-03 02:52:06 PDT
Fix checked into the 1.7.2 branch also
Comment 14 Asa Dotzler [:asa] 2004-09-13 11:13:18 PDT
Verified with the two testcases on Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.7.3) Gecko/20040910
Comment 15 Keith Bowes 2004-09-28 00:20:37 PDT
The test cases still work (i.e. the bug still exists) for me in Linux with the
GTK2/XFT nightly builds of both Mozilla and Firefox; however, it does seems
fixed with the GTK1 builds.
Comment 16 Johnny Stenback (:jst, 2004-09-28 17:26:03 PDT
Reopening, as this is still not fixed on linux.
Comment 17 Wladimir Palant 2004-09-29 00:21:32 PDT
Just a guess - is this because of bug 257405?
Comment 18 Wladimir Palant 2004-09-29 00:25:44 PDT
Looking at the patch - this seems to be the case. DOMEventToNativeKeyEvent()
should return PR_FALSE for non-trusted events.
Comment 19 Wladimir Palant 2004-09-29 06:03:55 PDT
Created attachment 160498 [details] [diff] [review]
Patch for the regression

This fixes the regression caused by bug 257405. AFAICT this is the only place
where we need a check.
Comment 20 Johnny Stenback (:jst, 2004-09-29 10:24:19 PDT
Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

Thanks for the quick fix, Wladimir!

Comment 21 Daniel Veditz [:dveditz] 2004-09-29 11:16:06 PDT
Comment on attachment 160498 [details] [diff] [review]
Patch for the regression


bug 257523 landed on the branches, so we'll want this regression fix there as
Comment 22 Mike Kaply [:mkaply] 2004-09-30 05:39:25 PDT
Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

a=mkaply for both
Comment 23 Johnny Stenback (:jst, 2004-09-30 09:32:45 PDT
Fixed on trunk, aviary and 1.7 branches. Thanks for the patch, Wladimir!
Comment 24 Tracy Walker [:tracy] 2004-12-16 13:06:46 PST
verified with windows 1.7.5 12/15
Comment 25 Marcia Knous [:marcia - use ni] 2004-12-16 13:47:28 PST
verified with Mac 1.75 2004-12-16-09
Comment 26 Biju 2007-11-17 10:26:41 PST
I only see Ctrl-Ins and Shift-Ins mentioned.

Are we taking care of "Shift-Del" which is equivalent to ctrl-x

Also if we can get a test case, it will be great help.
Comment 27 Wladimir Palant 2007-11-17 11:27:49 PST
The fix here is general and takes care of all keys handled by XBL bindings.

Note You need to log in before you can comment on or make changes to this bug.