257523 - Text fields give scripts access to the user's clipboard

Status: VERIFIED FIXED

(Core :: DOM: Events, defect)

Description

[Wladimir Palant]

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]

Attachment: Testcase - clipboard read

Comment 2

[Wladimir Palant]

Attachment: Testcase - clipboard write

Comment 3

[Wladimir Palant]

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)]

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).
<http://freedesktop.org/Standards/clipboards-spec/clipboards.txt>

Comment 5

[Wladimir Palant]

Attachment: 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)]

Comment on attachment 157608 [details] [diff] [review]
Proposed patch

Seems reasonable to me. r=jst

Comment 7

[Daniel Veditz [:dveditz]]

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.

sr=dveditz

Comment 8

[Daniel Veditz [:dveditz]]

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]

Comment on attachment 157608 [details] [diff] [review]
Proposed patch

a=chofmann for the branches

Comment 10

[timeless]

we run as chrome and i actually have keyhandling off. supporting midas is going
to be so much fun.

Comment 11

[Johnny Stenback (:jst)]

Attachment: Patch that was checked in

Comment 12

[Johnny Stenback (:jst)]

Fixed on aviary and 1.7 branches, and on the trunk. Marking FIXED.

Comment 13

[Daniel Veditz [:dveditz]]

Fix checked into the 1.7.2 branch also

Comment 14

[Asa Dotzler [:asa]]

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]

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)]

Reopening, as this is still not fixed on linux.

Comment 17

[Wladimir Palant]

Just a guess - is this because of bug 257405?

Comment 18

[Wladimir Palant]

Looking at the patch - this seems to be the case. DOMEventToNativeKeyEvent()
should return PR_FALSE for non-trusted events.

Comment 19

[Wladimir Palant]

Attachment: 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)]

Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

Thanks for the quick fix, Wladimir!

r=jst

Comment 21

[Daniel Veditz [:dveditz]]

Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

sr=dveditz

bug 257523 landed on the branches, so we'll want this regression fix there as
well.

Comment 22

[Mike Kaply [:mkaply]]

Comment on attachment 160498 [details] [diff] [review]
Patch for the regression

a=mkaply for both

Comment 23

[Johnny Stenback (:jst)]

Fixed on trunk, aviary and 1.7 branches. Thanks for the patch, Wladimir!

Comment 24

[Tracy Walker [:tracy]]

verified with windows 1.7.5 12/15

Comment 25

[Marcia Knous [:marcia]]

verified with Mac 1.75 2004-12-16-09

Comment 26

[Biju]

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]

The fix here is general and takes care of all keys handled by XBL bindings.