Ticket #7366 (closed Bug: fixed)

Opened 4 years ago

Last modified 3 years ago

Pasting from browser toolbar menu bypasses paste filters

Reported by: arne Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.6.3
Component: Core : Pasting Version: 3.0
Keywords: Cc: satya_minnekanti@…

Description (last modified by Anna) (diff)

  1. Go to demo page
  1. Paste some content (i.e. from MS Word) by choosing 'Edit' -> 'Paste' from the browser toolbar.
  1. Switch to source mode and observe the garbage HTML.

Tested in WinXP (FF 3.6.15, IE8)

Seems nothing is listening to the document paste event.

Attachments

7366.patch (5.7 KB) - added by Reinmar 3 years ago.
Fix for Firefox&IE
7366_2.patch (2.1 KB) - added by garry.yao 3 years ago.
7366_3.patch (5.8 KB) - added by Reinmar 3 years ago.
7366_4.patch (1.5 KB) - added by garry.yao 3 years ago.
7366_5.patch (2.2 KB) - added by garry.yao 3 years ago.
7366_6.patch (2.6 KB) - added by garry.yao 3 years ago.

Change History

comment:1 Changed 4 years ago by Anna

  • Status changed from new to confirmed
  • Component changed from General to Core : Pasting
  • Description modified (diff)

comment:2 Changed 4 years ago by satya

  • Cc satya_minnekanti@… added

comment:3 Changed 4 years ago by wwalc

Looks similar to #7304

comment:4 Changed 4 years ago by garry.yao

  • Status changed from confirmed to pending

Can you verify the bug is fixed on trunk?

comment:5 Changed 4 years ago by arne

Just pointing out that this relates to using the browser toolbar, not the editor toolbar, so in that it differs from #7304 it seems.

comment:6 Changed 4 years ago by wwalc

  • Status changed from pending to confirmed
  • Version set to 3.0

Indeed, using "Edit -> Paste" from the browser toolbar makes the difference here. Confirmed in IE8 / Win 7.

comment:7 Changed 3 years ago by garry.yao

Link 2.x bug #756.

comment:8 Changed 3 years ago by j.swiderski

Looks to me like quite a critical issue.

There are 3 ways to bypass filters.

  1. Using "Edit -> Paste"
  2. Using Native browser Menu - just use CRTL + Right-click.
  3. Using Drag and Drop from word directly to CKEditor.

All of those cases produce garbage HTML.

Issue has been reproducible in all browsers from CKEditor 3.0.

Changed 3 years ago by Reinmar

Fix for Firefox&IE

comment:9 Changed 3 years ago by Reinmar

  • Owner set to Reinmar
  • Status changed from confirmed to review

Changed 3 years ago by garry.yao

comment:10 Changed 3 years ago by garry.yao

  • Owner changed from Reinmar to garry.yao

7366.patch fails the following case:

  1. Click "Paste" from browser toolbar menu;
  2. On clipboard security popup choose "Allow";
    • Actual: Paste dialog opened;
    • Expected: Pasted succeed without dialog;

Besides, the patch is over-complicated, considering all we need is to invoke the editor paste command, providing a simplified one.

comment:11 Changed 3 years ago by Reinmar

Patch 7366_2.patch doesn't work for me (tested on IE8 & Fx):

  1. For quickly repeated ctrl+v IE throws security alert.
  2. For native context menu and native menu bar it doesn't filter MSWord stuff.
  3. Additionally - line endings differ from these in original plugin.js, so I had a problem with applying patch on Linux.

The way I'm testing (for v4 we've got playground and there it's easily visible if pasted content was handled by us, but for v3 we don't have it):

  1. Set editor data to: <p>This is more <strong class="MsoNormal">sample text</strong>.</p>
  2. Copy this in wysiwyg mode.
  3. Paste into the editor.
  4. Check in source mode if MsoNormal class was removed by filter.
Last edited 3 years ago by Reinmar (previous) (diff)

Changed 3 years ago by Reinmar

comment:12 Changed 3 years ago by Reinmar

7366.patch works for me, but it was missing one thing. For declined security alert opened by pasting from native context menu or menu bar it wasn't opening paste dialog. This is fixed by 7366_3.patch.

However I've checked now that this patch doesn't work with latest trunk version for pasting from menu bar. It throws some error related probably to dialog.

But with revision 7364 it works perfectly fine - all features are ok.

comment:13 Changed 3 years ago by garry.yao

  • Status changed from review to assigned

The trunk is right now unstable to review this ticket, #8843 and #8849 need to be reviewed before hand.

comment:14 Changed 3 years ago by garry.yao

But anyway 7366_3 is still too drastic a change for v3, since it changes the paste event used for other browsers as well, my intention is to provide a dedicated patch only for the toolbar menu paste issue, so a simplified patch must be considered.

comment:15 Changed 3 years ago by Reinmar

I believe there's another solution for all clipboard's problem. But I'm not sure it will be any simpler. The problem is caused by the fact that all browsers behave differently when they fire (before)paste events.

Anyway, I'm keeping finger crossed for you :)

comment:16 Changed 3 years ago by garry.yao

  • Status changed from assigned to review

Ok, I come with a latest patch that fixes the above tcs while it need to be reviewed along with changes proposed in #8843 and #8849 (in order to have correct cursor positon).

Changed 3 years ago by garry.yao

comment:17 Changed 3 years ago by Reinmar

I've checked 7366_4.patch and it seems to work correctly (I wasn't checking against #8843 and #8849). It stripped some features, but not critical ones. And it's definitely shorter :>. Gr8 job.

comment:18 Changed 3 years ago by garry.yao

  • Milestone set to CKEditor 3.6.3

Since it's almost reviewed, let's consider it for the release.

comment:19 Changed 3 years ago by spoutnik

7366_4 is working like a charm (ubuntu ff11, winxp ff11, winxp ie8, win7 ie9, win7 ff10)

comment:20 Changed 3 years ago by garry.yao

  • Status changed from review to review_failed

The patch forces the paste dialog on Fx and Webkit which can be avoided.

Changed 3 years ago by garry.yao

comment:21 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

New patch should have exactly the same code path for IE (so it doesn't have to be retested on IE ) while fix the above mentioned issue.

comment:22 Changed 3 years ago by Reinmar

I've found an issue on IE8.

  1. click edit->paste (in native menu bar)
  2. IE throws security alert
  3. accept it
  4. paste dialog will open
  5. click edit->paste again
  6. content will be pasted correctly

The same happens for ckeditor's context menu.

Maybe this is caused by ckeditor's commands which still fires the same native events than before, when now IE listens for different one. Note that in my patch I moved event name to the mainPasteEvent variable.

Last edited 3 years ago by Reinmar (previous) (diff)

comment:23 Changed 3 years ago by j.swiderski

#8867 was marked as duplicate

comment:24 Changed 3 years ago by fredck

Other than the above, I've noticed that CTRL+RIGHT-CLICK is not opening the security alert on IE9, which seems to be wrong (considering that we're not able to grab the "paste" event in time with IE).

comment:26 Changed 3 years ago by fredck

  • Status changed from review to review_failed

Changed 3 years ago by garry.yao

comment:27 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

I'm unable to reproduce the tc in comment 22 with a default IE security configuration.

comment:28 Changed 3 years ago by fredck

  • Status changed from review to review_passed

comment:29 Changed 3 years ago by garry.yao

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [7453].

Last edited 3 years ago by garry.yao (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy