Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tpl: add sanitizeHTML template func #1457

Closed
anthonyfok opened this Issue Sep 24, 2015 · 13 comments

Comments

Projects
None yet
5 participants
@anthonyfok
Copy link
Contributor

commented Sep 24, 2015

Priority: wishlist

I was reviewing Blackfriday’s changelog, and came upon this entry:

commit cf6bfc9d6d9f0d0279ff7660e0095b21b7df8c86
Author: Vytautas Saltenis <vytas@rtfb.lt>
Date:   Fri Sep 19 20:30:00 2014 +0300

    Rip off all blackfriday's html sanitization effort

    As per discussion in issue #90.

Basically, according to russross/blackfriday#90, Blackfriday’s old HTML sanitization code wasn't doing it correctly, and it was decided that Blackfriday should follow the UNIX philosophy rule "to do one thing and do it well", and leave the sanitization to a specialized Go library, i.e.:

  • blackfriday is for parsing Markdown.
  • bluemonday is for cleaning untrusted HTML so that it can appear on a trusted web page.

So, I am wondering if anyone needs this HTML sanitization feature in Hugo?
The Blackfriday README.md now includes the following recommendation with sample code:

Sanitize untrusted content

Blackfriday itself does nothing to protect against malicious content. If you are
dealing with user-supplied markdown, we recommend running blackfriday's output
through HTML sanitizer such as Bluemonday.

Here's an example of simple usage of blackfriday together with bluemonday:

import (
    "github.com/microcosm-cc/bluemonday"
    "github.com/russross/blackfriday"
)

// ...
unsafe := blackfriday.MarkdownCommon(input)
html := bluemonday.UGCPolicy().SanitizeBytes(unsafe)

If anyone needs this or wants this, please speak up!
Or better yet, please submit a Pull Request! :-)

@bep

This comment has been minimized.

Copy link
Member

commented Sep 24, 2015

I should be optional and default off.

@anthonyfok anthonyfok added the Wish label Oct 13, 2015

@bep bep added the Stale label Feb 27, 2017

@bep

This comment has been minimized.

Copy link
Member

commented Mar 1, 2017

Note/Update: This issue is marked as stale, and I may have said something earlier about "opening a thread on the discussion forum". Please don't.

If this is a bug and you can still reproduce this error on the latest release or the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.

@dmitshur

This comment has been minimized.

Copy link

commented Mar 18, 2017

Does hugo operate on any external user input which might be malicious? If so, sanitizing unsafe HTML output of blackfriday via bluemonday needs to be done for safety. If the user input is trusted (e.g., own blog posts), then it's not neccessary for safety.

@bep

This comment has been minimized.

Copy link
Member

commented Mar 18, 2017

Does hugo operate on any external user input which might be malicious?

No, with a disclaimer: The user (i.e. the website creator) have plenty of opportunities to shoot him/herself in the foot if he/she wants to "be malicious".

This isn't high priority, which is the reason this issue has been left untouched for so long.

@moorereason

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2017

Simplest solution would be to offer a sanitizeHTML template function to frontend Bluemonday's UGCPolicy. Then the user could sanitize as needed.

@bep bep removed Stale Wish labels Mar 19, 2017

@bep bep changed the title Blackfriday no longer does HTML sanitization: Consider the use of Bluemonday in Hugo? tpl: add sanitizeHTML template func Mar 19, 2017

@kaushalmodi

This comment has been minimized.

Copy link
Member

commented Oct 25, 2017

I was running Google Chrome Inspector -> Lighthouse, when I came across the security suggestion to always have rel="noopener" for all target="_blank" links.

That led me to russross/blackfriday#343, and from there to here.

@jhabdas Were you able to make any progress on this?


I am forced to follow a bad practice like this as those _blank links are missing that rel property.

@kaushalmodi

This comment has been minimized.

Copy link
Member

commented Oct 26, 2017

@jhabdas Yes, the PR you linked is what led me here. My question was if there's any plan to add that noopener on Hugo side, as Blackfriday folks (discussion in that same PR) think that it's not a good idea to do that there.

@stale

This comment has been minimized.

Copy link

commented Jan 24, 2018

This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

@stale stale bot added the Stale label Jan 24, 2018

@kaushalmodi

This comment has been minimized.

Copy link
Member

commented Jan 24, 2018

It would be great if Hugo can add the rel=noopener attribute. As it stands, all I get is radio silence on the Blackfriday repo.

@stale stale bot removed the Stale label Jan 24, 2018

moorereason added a commit to moorereason/hugo that referenced this issue Feb 8, 2018

@stale

This comment has been minimized.

Copy link

commented May 24, 2018

This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

@stale stale bot added the Stale label May 24, 2018

@stale stale bot closed this Jun 23, 2018

@anthonyfok anthonyfok reopened this Jun 23, 2018

@stale stale bot removed the Stale label Jun 23, 2018

@anthonyfok

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2018

Hi @moorereason, thank you for working on this issue! I saw that you committed 761f567 to moorereason/hugo in February 2018, but I couldn't find a corresponding pull request. Could you please enlighten me? Many thanks!

@moorereason

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2018

@anthonyfok,
I think the commit in my branch was a proof-of-concept and then I forgot to follow up on it. I'll try to pick it back up this week and submit a PR. Thanks for the prodding. 👍

@stale

This comment has been minimized.

Copy link

commented Nov 18, 2018

This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

@stale stale bot added the Stale label Nov 18, 2018

@stale stale bot closed this Dec 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.