Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#44724 closed enhancement (fixed)

KSES: Allow 'download' attribute for links

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: chriscct7's profile chriscct7
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Formatting Keywords: has-patch has-unit-tests fixed-5.0
Focuses: Cc:

Description

HTML5 introduced a download attribute for <a> tag. When a user with Author role tries to create a link with this attribute, it's stripped by KSES.

Before saving:

<a href="/images/w3logo.jpg" download="w3logo">w3logo</a>

After saving:

<a href="/images/w3logo.jpg">w3logo</a>

At the moment, allowed attributes for <a> are: href, rel, rev, name, and target.

download should be added as well.

Attachments (4)

44724.patch (365 bytes) - added by kalpshit 6 years ago.
I added download attribute in kese( allowed post tags)
44724.1.patch (630 bytes) - added by arshidkv12 6 years ago.
Added spacing
44724.diff (1.9 KB) - added by welcher 6 years ago.
Adding unit test
44724-50-branch.diff (721 bytes) - added by peterwilsoncc 6 years ago.

Download all attachments as: .zip

Change History (19)

#1 @SergeyBiryukov
6 years ago

  • Milestone changed from 5.0 to 4.9.9

@kalpshit
6 years ago

I added download attribute in kese( allowed post tags)

#2 @kalpshit
6 years ago

  • Focuses coding-standards added
  • Keywords has-patch added; needs-patch removed

@arshidkv12
6 years ago

Added spacing

#3 @chriscct7
6 years ago

  • Focuses coding-standards removed
  • Keywords needs-unit-tests added
  • Type changed from defect (bug) to enhancement

#4 @chriscct7
6 years ago

  • Owner set to chriscct7
  • Status changed from new to accepted

#5 @chriscct7
6 years ago

Will work on some unit tests for this, and then it'll be ready for merge.

#6 @SergeyBiryukov
6 years ago

When committing, props should also be given to @marina_wp for reporting the issue.

#7 @welcher
6 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

@welcher
6 years ago

Adding unit test

#8 @pento
6 years ago

  • Milestone changed from 4.9.9 to 5.0

#9 @peterwilsoncc
6 years ago

44724-50-branch.diff is identical to previous patch reformatted to apply cleanly to the 5.0 branch.

#10 @pento
6 years ago

  • Keywords commit added

👍🏻

#11 @pento
6 years ago

  • Keywords good-first-bug commit removed

...not so fast. 😔

The download attribute doesn't work on cross-origin links (eg, any site that uses a CDN for hosting uploads). I don't know that we necessarily need to account for this, but it is something to consider.

It's also a risk to allow the download filename to be set: for example, an author could upload my_definitely_not_suspicious_file.txt, but then set the download attribute to be CLICK_ME.bat, which isn't great. If we do allow the download attribute, it should only be allowed with no value.

#12 @pento
6 years ago

We don't need to remove the download attribute entirely. If we just restrict it to being set (but not given a value), that removes the security issues.

For sites that use a CDN for hosting uploads, it's possible touse a file passthrough handler to add the Content-Disposition: attachment header, forcing the file to be a download.

This ticket doesn't need an update until GB#10693 is resolved, which will change the behaviour of the file block to match.

#13 @pento
6 years ago

In 43813:

KSES: Allow the download attribute on <a> tags.

To avoid this being a vector for bypassing the filetypes that are allowed to be uploaded, this attribute is only allowed to be added without a value.

Props kalpshit, arshidkv12, welcher, peterwilsoncc, marina_wp, pento.
See #44724.

#14 @pento
6 years ago

  • Keywords fixed-5.0 added

#15 @pento
6 years ago

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

In 44156:

KSES: Allow the download attribute on <a> tags.

To avoid this being a vector for bypassing the filetypes that are allowed to be uploaded, this attribute is only allowed to be added without a value.

Merges [43813] from the 5.0 branch to trunk.

Props kalpshit, arshidkv12, welcher, peterwilsoncc, marina_wp, pento.
Fixes #44724.

Note: See TracTickets for help on using tickets.