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

feat: Process XSS security problem of each input box in the background #1883

Closed
wants to merge 9 commits into from

Conversation

Ljfanny
Copy link

@Ljfanny Ljfanny commented Apr 23, 2022

What this PR does?

fix #1557

Mechanism

  • 加入了XSSfilter,该Filter会对原生Http请求做一次包装。其中对于Parameter类型参数,逐一进行ESCAPE_HTML4编码绕过script脚本执行。对于json类型RequestBody的参数,在springboot内置的json处理框架jackson里注册了转换器对value进行ESCAPE_HTML4编码。
  • 可以在application.yaml里配置是否启动xss检查,并且可以设置跳过检查的url(暂未测试)。

Problem

  • markdown转html的正文,如日志和文章等(其对应的字段都为content)没法直接对其用html格式编码。目前的权宜之计是对该字段进行特判绕过编码。对originalContent字段进行ESCAPE_HTML4编码后再后端调用renderHtml后把结果赋给content,这个需要前端传来的keepRaw字段为false。

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Apr 23, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign ruibaby after the PR has been reviewed.
You can assign the PR to them by writing /assign @ruibaby in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@CLAassistant
Copy link

CLAassistant commented Apr 23, 2022

CLA assistant check
All committers have signed the CLA.

@ruibaby
Copy link
Member

ruibaby commented Apr 24, 2022

@Ljfanny 感谢你的贡献,但我认为此修改在现阶段并不适合添加。因为据原 issue 所说,目前属于单管理员的模式,理论上不会造成安全问题,除非管理员有意为之,但这和直接修改代码没有区别。而且在目前有部分需求是需要在文章中插入 iframe 或者 script 标签的。

对originalContent字段进行ESCAPE_HTML4编码后再后端调用renderHtml后把结果赋给content,这个需要前端传来的keepRaw字段为false。

目前 1.5.x 已经不再使用后端渲染 Markdown,而是直接保存前端编辑器渲染的 HTML,这是为了编辑预览和实际保存的效果一致,所以可能并不适合修改。

@ruibaby
Copy link
Member

ruibaby commented Apr 24, 2022

另外,在提交 PR 前,我们希望先在原 issue 进行方案讨论。

/hold

@f2c-ci-robot f2c-ci-robot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2022
@Ljfanny
Copy link
Author

Ljfanny commented Apr 24, 2022

@Ljfanny 感谢你的贡献,但我认为此修改在现阶段并不适合添加。因为据原 issue 所说,目前属于单管理员的模式,理论上不会造成安全问题,除非管理员有意为之,但这和直接修改代码没有区别。而且在目前有部分需求是需要在文章中插入 iframe 或者 script 标签的。

对originalContent字段进行ESCAPE_HTML4编码后再后端调用renderHtml后把结果赋给content,这个需要前端传来的keepRaw字段为false。

目前 1.5.x 已经不再使用后端渲染 Markdown,而是直接保存前端编辑器渲染的 HTML,这是为了编辑预览和实际保存的效果一致,所以可能并不适合修改。

明白您的意思,虽然目前单用户的情况下的xss注入确实不是什么大问题,但是考虑到如果博客的维护者将一些恶意脚本加进博客(或者管理者不知情的情况下引入),也有可能对正常的访客造成影响。 确实考虑到一些用户有在正文里插入一些标签的需求,可以考虑对markdown语法的输入框不做处理,只对其他的输入框做处理,比如文章标题等。
另外对没有先讨论issue的事情也感到很抱歉,因为课程原因需要为开源社区贡献pr,第一次操作还不是很熟练,初衷也是想提供一个思路,完善一下安全问题。

Ljfanny and others added 2 commits April 29, 2022 20:34
统一了一下,都使用HtmlUtils.htmlEscape进行编码。
不再对markdown语法的内容(content,originalcontent 字段)进行编码。
@Yhcrown
Copy link

Yhcrown commented Apr 30, 2022

目前这个CI没法通过,image
里面的&会被转义,暂时也没解决办法。

@ruibaby
Copy link
Member

ruibaby commented Mar 7, 2023

由于 1.x 的版本已经进入维护阶段,不会添加新功能,并且此 PR 也已经不适用于 2.x,所以我将关闭此 PR。

再次感谢你的贡献。

/close

@f2c-ci-robot f2c-ci-robot bot closed this Mar 7, 2023
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Mar 7, 2023

@ruibaby: Closed this PR.

In response to this:

由于 1.x 的版本已经进入维护阶段,不会添加新功能,并且此 PR 也已经不适用于 2.x,所以我将关闭此 PR。

再次感谢你的贡献。

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

处理后台各个输入框的 XSS 安全问题
4 participants