WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#34575 closed feature request (invalid)

shortcode not working in html comments

Reported by: distinct Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.3.1
Component: Shortcodes Keywords:
Focuses: Cc:
PR Number:

Description

First of all I would like to mention that I have read https://make.wordpress.org/core/2015/09/04/shortcode-roadmap-extended-discussion/ so I know about the change to the shortcode system. What I could not find was the real security problem mentioned there. I would really like to know the CVE.

Now for the problem, I would like to be able to do this:

<table>
<!-- [conditional1] --><tr><td>row1</td></tr><!-- [/conditional1] -->
<!-- [conditional2] --><tr><td>row2</td></tr><!-- [/conditional2] -->
<!-- [conditional3] --><tr><td>row3</td></tr><!-- [/conditional3] -->
</table>

This way the table is editable in the visual tab and the html will be correct.

The way we fix this now is by only using the Text tab and the following html:

<table>
[conditional1]<tr><td>row1</td></tr>[/conditional1]
[conditional2]<tr><td>row2</td></tr>[/conditional2]
[conditional3]<tr><td>row3</td></tr>[/conditional3]
</table>

This does work with the new shortcode parsing, but the visual tab will wrap the shortcodes in <p>'s which will break the table.

#23786 mentions the inverse of this bug, so at some point this still worked. But now <!-- is explicitly skipped in do_shortcodes_in_html_tags

Thanks

Attachments (1)

shortcodes.php.diff (967 bytes) - added by distinct 4 years ago.
possible solution to allow shortcodes in <!-- and <![CDATA[

Download all attachments as: .zip

Change History (6)

@distinct
4 years ago

possible solution to allow shortcodes in <!-- and <![CDATA[

#1 follow-up: @miqrogroove
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

There are no bugs described in this ticket. You stated shortcodes do not work when escaped by HTML comments. That is correct.

Last edited 4 years ago by miqrogroove (previous) (diff)

#2 in reply to: ↑ 1 @distinct
4 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Type changed from defect (bug) to feature request

Replying to miqrogroove:

There are no bugs described in this ticket. You stated shortcodes do not work when escaped by HTML comments. That is correct.

Sorry, in my eyes a breaking feature change can be seen as a bug. But let's not go into semantics ;).

I hope it is ok if I reopen this as a feature request then.

#3 @miqrogroove
4 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

#4 @aaroncampbell
4 years ago

Hey @distinct, I just wanted to weigh in here and help explain why this isn't something that we're going to change. The full explanation is long and involved, having to do with the history of shortcodes, the mistakes that were made there, as well as all the various potential attack vectors they present and which ones are more or less in our control to fix. I'm going to try to keep this simple and short here though, because there has already been much discussion around it that you can find and read.

The problem comes into view when a user that should not be able to use certain HTML (such as a contributor) uses a shortcode that generates certain HTML. This is because the checks that limit that user's content happen on save and shortcodes aren't replaced with content at that point. The content of the shortcode is only handled on DISPLAY, so we have to deal with the content at that point. The most dangerous possible vectors there come from inserting code that is INSIDE HTML tags (including HTML comments). This is why we can't change to parse shortcodes inside HTML comments or tags.

I understand that you have a usecase where it would be nice to use a shortcode as a solution, but we can't safely have shortcodes do that so you'll need to find another way to do what you want.

#5 @distinct
4 years ago

Hi @aaroncampbell, thank you for your more informed reply. This helps a bit to put the changes into context.

But I still have not found any real details about the actual security risk. And I noticed that the link in my OP was actually not the article I read :$, but another one I was about to read but forgot about. I actually meant this link: https://make.wordpress.org/core/2015/07/23/changes-to-the-shortcode-api/
There Dave Navarro, Jr. mentions he could not find any details later on the security risk either at first, but later mentions Gary Pendergast and others have explained it to him, without any link to the real problem. I would really like to know what the actual security risk is, because I think not all sites will have it. (our site has no contributors for instance, only a few select authors that can edit content)

I will read further into the coming shortcode changes. But I fear that they might force me to look into another templating engine for this feature. Which is a shame, because the shortcode system was working fine for the simple functions we needed.

I do realize that it might be considered misuse of a feature to really use it as a templating language. And it is certainly up for some improvements. But I hope that the improvements don't have to weaken the power that it used to have.

Note: See TracTickets for help on using tickets.