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

safeHTML use should be avoided #134

Closed
palant opened this issue May 12, 2020 · 2 comments · Fixed by #135
Closed

safeHTML use should be avoided #134

palant opened this issue May 12, 2020 · 2 comments · Fixed by #135
Labels
enhancement New feature or request
Milestone

Comments

@palant
Copy link
Collaborator

palant commented May 12, 2020

The theme currently uses safeHTML function a lot. This is very undesirable security-wise, and it is hard to verify that no XSS-vulnerabilities are introduced here. With merely few safeHTML calls reviewing its security implications would be much simpler.

For example in post-meta.html:

    {{ printf `<time datetime="%s" class="post-meta-item published">%s&nbsp;%s</time>` ($Deliver.PublishDate.Format "2006-01-02T15:04:05-07:00") $icon ($Deliver.PublishDate.Format $Deliver.Site.Params.postMetaDateFormat) | safeHTML }}

This should be:

    <time datetime="{{ $Deliver.PublishDate.Format "2006-01-02T15:04:05-07:00" }}" class="post-meta-item published">{{ $icon | safeHTML }}&nbsp;{{ $Deliver.PublishDate.Format $Deliver.Site.Params.postMetaDateFormat }}</time>

That's both safer (safeHTML applied only to the icon where it makes sense) and more readable. In fact, it might be a good idea to create a template for icons, so that safeHTML can be applied to icons in that one place only (as an added bonus, this template could replace the icon class as well).

Or in footer.html:

{{- partial "utils/markdownify.html" (dict "Deliver" $Deliver "raw" $raw "isContent" false) -}}
{{- $Content := .Scratch.Get "Content" -}}
<div class="powered-by">{{ $Content | safeHTML }}</div>

Here safeHTML should be removed because markdownify.html already applies it (either directly, or via markdownify function).

@palant
Copy link
Collaborator Author

palant commented May 12, 2020

I could create a pull request covering most of the necessary changes (the ones that I can test) if you are ok with that.

@reuixiy
Copy link
Owner

reuixiy commented May 12, 2020

Yes, absolutely welcome!

I realized this problem after I finished this theme, but I was too lazy to check them one by one.

Feel free to fix these safeHTMLs :)

@reuixiy reuixiy added the enhancement New feature or request label May 12, 2020
@reuixiy reuixiy added this to the v4.4.0 milestone May 12, 2020
reuixiy added a commit that referenced this issue May 14, 2020
Fixes #134.

* Simplify post-meta.html template logic

* Fixed indentation in previous commit

* Use icon.html template for zodiac icons in lists

* Remove unnecessary safeHTML usage in footer.html (busuanzi functionality unchanged, cannot test)

* Improved previous commit, no unnecessary markup for the copyright symbol

* Remove unnecessary safeHTML usage in style.html

* Use icon.html partial in the header

* Remove unnecesary safeHTML usage in script.html

* Removed unnecessary safeHTML usage in menu.html

* Remove unnecessary safeHTML use in header.html

* Use icon.html partial in back-to-top and port-share components

* Use icon.html partial in related-posts and socials components

* Remove unnecessary safeHTML use in feed generation

* Simplify minimal-footer-about component

* Removed safeHTML usage from post-updated-badge component

* Remove unnecessary safeHTML usage in post-copyright component

* Remove unnecessary safeHTML usage in post-nav component

* Simplify minimal-footer component and remove safeHTML usage

* Simplify post-tags component and remove safeHTML usage

* Made summary.html produce its result directly rather than via scratch, removed unnecessary safeHTML usage in home-posts.html

* Use icon.html partial on home-footage page

* Remove unnecessary safeHTML usage on post.html page

* Remove unnecessary safeHTML usage on home-poetry.html page

* Remove unnecessary safeHTML usage from OpenGraph data

* Remove unnecessary safeHTML usage from utils/list-item.html

* Improve image autodetection regex and remove unnecessary unsafeHTML usage in images.html

* Remove unnecessary safeHTML usage from relative-url.html

* Remove unnecessary safeHTML usage in content.html

* Revert image auto-detection changes, these aren't useful

* Delete the missing line in post-share

* Revert to using urls.Parse on tag and category names passed to .GetPage again

* Fix error caused by wrong context in list.html

* Replace hyphen with en dash

* Remove unnecessary safeHTML usage in post-gitinfo.html

* Remove unnecessary safeHTML usage of busuanzi in footer.html

Co-authored-by: reuixiy <reuixiy@gmail.com>
ulmefors pushed a commit to ulmefors/hugo-theme-meme that referenced this issue Nov 22, 2020
Fixes reuixiy#134.

* Simplify post-meta.html template logic

* Fixed indentation in previous commit

* Use icon.html template for zodiac icons in lists

* Remove unnecessary safeHTML usage in footer.html (busuanzi functionality unchanged, cannot test)

* Improved previous commit, no unnecessary markup for the copyright symbol

* Remove unnecessary safeHTML usage in style.html

* Use icon.html partial in the header

* Remove unnecesary safeHTML usage in script.html

* Removed unnecessary safeHTML usage in menu.html

* Remove unnecessary safeHTML use in header.html

* Use icon.html partial in back-to-top and port-share components

* Use icon.html partial in related-posts and socials components

* Remove unnecessary safeHTML use in feed generation

* Simplify minimal-footer-about component

* Removed safeHTML usage from post-updated-badge component

* Remove unnecessary safeHTML usage in post-copyright component

* Remove unnecessary safeHTML usage in post-nav component

* Simplify minimal-footer component and remove safeHTML usage

* Simplify post-tags component and remove safeHTML usage

* Made summary.html produce its result directly rather than via scratch, removed unnecessary safeHTML usage in home-posts.html

* Use icon.html partial on home-footage page

* Remove unnecessary safeHTML usage on post.html page

* Remove unnecessary safeHTML usage on home-poetry.html page

* Remove unnecessary safeHTML usage from OpenGraph data

* Remove unnecessary safeHTML usage from utils/list-item.html

* Improve image autodetection regex and remove unnecessary unsafeHTML usage in images.html

* Remove unnecessary safeHTML usage from relative-url.html

* Remove unnecessary safeHTML usage in content.html

* Revert image auto-detection changes, these aren't useful

* Delete the missing line in post-share

* Revert to using urls.Parse on tag and category names passed to .GetPage again

* Fix error caused by wrong context in list.html

* Replace hyphen with en dash

* Remove unnecessary safeHTML usage in post-gitinfo.html

* Remove unnecessary safeHTML usage of busuanzi in footer.html

Co-authored-by: reuixiy <reuixiy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants