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

bpo-35603: Escape table header of make_table output that can cause potential XSS #11341

Merged
merged 3 commits into from Dec 29, 2018

Conversation

@tirkarthi
Copy link
Contributor

commented Dec 28, 2018

  • fromdesc and todesc should be escaped since rendering unescaped HTML might lead to code execution in the browser rendering them as tags.

https://bugs.python.org/issue35603

@@ -2036,6 +2036,10 @@ def make_table(self,fromlines,tolines,fromdesc='',todesc='',context=False,
s.append( fmt % (next_id[i],next_href[i],fromlist[i],
next_href[i],tolist[i]))
if fromdesc or todesc:
fromdesc = fromdesc.replace("&", "&").replace(">", ">") \

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Dec 28, 2018

Member

Why not use html.escape()?

This comment has been minimized.

Copy link
@tirkarthi

tirkarthi Dec 28, 2018

Author Contributor

I thought about the same but text which was encoded was using the logic inlined and there was some discussion in the issue about just inlining the logic to avoid dependency at https://bugs.python.org/issue914575#msg45518 which I realize now is about xml.sax and not html.

html module was also importing entities at

from html.entities import html5 as _html5
that seemed little import heavy for me. But if it's okay to import html then I think it's good to use html.escape across the module to avoid duplicate code.

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Dec 28, 2018

Member

Well, for a bug fix we can keep an inlined code and consider using html.escape() in a separate issue.

This comment has been minimized.

Copy link
@tirkarthi

tirkarthi Dec 28, 2018

Author Contributor

Sure, thanks for the review.

@@ -2036,6 +2036,10 @@ def make_table(self,fromlines,tolines,fromdesc='',todesc='',context=False,
s.append( fmt % (next_id[i],next_href[i],fromlist[i],
next_href[i],tolist[i]))
if fromdesc or todesc:
fromdesc = fromdesc.replace("&", "&").replace(">", ">") \

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Dec 28, 2018

Member

Well, for a bug fix we can keep an inlined code and consider using html.escape() in a separate issue.

@serhiy-storchaka serhiy-storchaka merged commit 78de011 into python:master Dec 29, 2018

5 checks passed

Azure Pipelines PR #20181228.14 succeeded
Details
bedevere/issue-number Issue number 35603 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Copy link

commented Dec 29, 2018

Thanks @tirkarthi for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒🤖 I'm not a witch! I'm not a witch!

@bedevere-bot

This comment has been minimized.

Copy link

commented Dec 29, 2018

GH-11353 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 29, 2018
bpo-35603: Escape table header of make_table output that can cause po…
…tential XSS. (pythonGH-11341)

(cherry picked from commit 78de011)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
@bedevere-bot

This comment has been minimized.

Copy link

commented Dec 29, 2018

GH-11354 is a backport of this pull request to the 3.6 branch.

miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 29, 2018
bpo-35603: Escape table header of make_table output that can cause po…
…tential XSS. (pythonGH-11341)

(cherry picked from commit 78de011)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
serhiy-storchaka added a commit that referenced this pull request Dec 29, 2018
serhiy-storchaka added a commit that referenced this pull request Jan 2, 2019
arnolddumas added a commit to arnolddumas/cpython that referenced this pull request May 3, 2019
arnolddumas added a commit to arnolddumas/cpython that referenced this pull request May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.