Bug 178422 – Don't expose raw HTML in pasteboard to the web content

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

HTML in the system pasteboard may contain privacy sensitive information such has file URLs.
Use the HTML sanitization mechanism added in https://trac.webkit.org/r223440 to do this.

Also, start using blob URLs in the pasted images.

[Ryosuke Niwa]

Created attachment 324081 [details]
WIP

[Build Bot]

Attachment 324081 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:166:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 324081 [details]
WIP

Attachment 324081 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4894046

New failing tests:
http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html

[Build Bot]

Created attachment 324087 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 324081 [details]
WIP

Attachment 324081 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4894261

New failing tests:
fast/events/ondrop-text-html.html

[Build Bot]

Created attachment 324092 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 324081 [details]
WIP

Attachment 324081 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4894253

New failing tests:
fast/events/ondrop-text-html.html

[Build Bot]

Created attachment 324094 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 324081 [details]
WIP

Attachment 324081 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4894263

New failing tests:
editing/pasteboard/copy-paste-wraps-position-absolute.html
editing/pasteboard/paste-text-013.html
editing/pasteboard/paste-double-nested-blockquote-before-blockquote.html
editing/pasteboard/paste-blockquote-and-paragraph-break.html
editing/pasteboard/paste-list-003.html
editing/pasteboard/paste-text-012.html
editing/pasteboard/paste-list-002.html
editing/pasteboard/paste-text-016.html
editing/pasteboard/paste-text-014.html
editing/pasteboard/paste-text-017.html
editing/pasteboard/copy-paste-inserts-clearing-div.html
editing/pasteboard/block-wrappers-necessary.html
editing/pasteboard/paste-blockquote-before-blockquote.html
editing/pasteboard/copy-paste-converts-fixed.html
editing/pasteboard/paste-table-001.html
editing/pasteboard/4922709.html
editing/pasteboard/pasting-into-p-should-not-nest-p.html
editing/pasteboard/paste-text-005.html
editing/pasteboard/paste-table-003.html
editing/pasteboard/data-transfer-get-data-on-paste-rich-text.html
editing/pasteboard/paste-text-004.html
editing/pasteboard/paste-text-006.html

[Build Bot]

Created attachment 324095 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Ryosuke Niwa]

<rdar://problem/34567052>

[Ryosuke Niwa]

Comment on attachment 324081 [details]
WIP

Obviously not up for a review.

[Ryosuke Niwa]

Created attachment 324098 [details]
Implements the feature

[Ryosuke Niwa]

Created attachment 324100 [details]
Fixed the test

[Wenson Hsieh]

Comment on attachment 324100 [details]
Fixed the test

View in context: https://bugs.webkit.org/attachment.cgi?id=324100&action=review

> Source/WebCore/editing/WebContentReader.cpp:44
>      return frame.document() && frame.document()->originIdentifierForPasteboard() != contentOrigin;

I'm not sure checking frame.document() is needed here, since we just assume document exists (*frame.document()) afterwards in both branches.

> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:288
> +    [representationsToRegister addData:customData.createSharedBuffer()->createNSData().get() forType:@(PasteboardCustomData::cocoaType())];

Just to make sure — it looks like we don't need to worry about also adding this to teamData here because we only need to know the origin for markup sanitization on drop?

> Source/WebCore/platform/mac/PasteboardWriter.mm:121
> +        [pasteboardItem setData:customData.createSharedBuffer()->createNSData().get() forType:toUTIUnlessAlreadyUTI(String(PasteboardCustomData::cocoaType())).get()];

Do we need to go through toUTIUnlessAlreadyUTI() here? We already know PasteboardCustomData::cocoaType() is a custom type (not one of the declared CoreServices UTIs).

> LayoutTests/http/tests/misc/copy-resolves-urls.html:45
> +    results.appendChild(document.createTextNode(pasteHere.innerHTML.replace(/blob\:http\:\/\/localhost\:8080\/[a-z0-9\-]+/, 'blob:://localhost:8080/...')));

Nit - extra : after the blob: here.

> LayoutTests/http/tests/security/clipboard/drag-drop-html-cross-origin-iframe-in-same-origin.html:24
> +setTimeout(finishJSTest, 3000);

I'm guessing this was just for debugging?

[Ryosuke Niwa]

Thanks for the review.

(In reply to Wenson Hsieh from comment #15)
> Comment on attachment 324100 [details]
> Fixed the test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324100&action=review
> 
> > Source/WebCore/editing/WebContentReader.cpp:44
> >      return frame.document() && frame.document()->originIdentifierForPasteboard() != contentOrigin;
> 
> I'm not sure checking frame.document() is needed here, since we just assume
> document exists (*frame.document()) afterwards in both branches.

Good point. Replaced it with an assertion.
 
> > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:288
> > +    [representationsToRegister addData:customData.createSharedBuffer()->createNSData().get() forType:@(PasteboardCustomData::cocoaType())];
> 
> Just to make sure — it looks like we don't need to worry about also adding
> this to teamData here because we only need to know the origin for markup
> sanitization on drop?

Yes, we don't need to put the type into the team data.

> > Source/WebCore/platform/mac/PasteboardWriter.mm:121
> > +        [pasteboardItem setData:customData.createSharedBuffer()->createNSData().get() forType:toUTIUnlessAlreadyUTI(String(PasteboardCustomData::cocoaType())).get()];
> 
> Do we need to go through toUTIUnlessAlreadyUTI() here? We already know
> PasteboardCustomData::cocoaType() is a custom type (not one of the declared
> CoreServices UTIs).

So LocalPasteboard.mm in DumpRenderTree has an assertion for checking that either UTI is a registered type or a dynamic type, and we have to call toUTIUnlessAlreadyUTI in order to not hit that assertion,

> > LayoutTests/http/tests/misc/copy-resolves-urls.html:45
> > +    results.appendChild(document.createTextNode(pasteHere.innerHTML.replace(/blob\:http\:\/\/localhost\:8080\/[a-z0-9\-]+/, 'blob:://localhost:8080/...')));
> 
> Nit - extra : after the blob: here.

Oops, fixed.

> > LayoutTests/http/tests/security/clipboard/drag-drop-html-cross-origin-iframe-in-same-origin.html:24
> > +setTimeout(finishJSTest, 3000);
> 
> I'm guessing this was just for debugging?

Yup, removed.

[Ryosuke Niwa]

Committed r223678: <https://trac.webkit.org/changeset/223678>