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

要不要考虑换一个markdown解析库 #440

Closed
albertshaw opened this issue Oct 28, 2014 · 25 comments
Closed

要不要考虑换一个markdown解析库 #440

albertshaw opened this issue Oct 28, 2014 · 25 comments
Assignees
Labels

Comments

@albertshaw
Copy link

之前报了一个marked的xss问题,markedjs/marked#492, 不过因为是特定浏览器下的反射型xss,所以我自己也没太在意(在cnodejs也提过,估计你们也不在意这个),隔了这么久才想起来看他们解决了没,却发现仍然是open的。

然后发现这个issue被NodeBB/nodebb-plugin-markdown#20 引用了,顺着过去看才发现marked问题不少,导致nodebb的markdown插件换了个解析库https://github.com/jonschlinkert/remarkable。

其中一个问题markedjs/marked#497 还蛮严重的,导致nodebb直接无响应了 https://github.com/akhoury/nodebb-plugin-import/issues/61。

@dead-horse
Copy link
Member

可以考虑, marked 的更新基本已经停滞了

@alsotang
Copy link
Member

@Ricardo-Li 把我们的 marked 换成这个 remarkable,有几点需要注意。

  1. 支持 GFM
  2. 忽略 marked 中的 html 语法,以防 xss。现有的 marked 对 code 标签进行了定制,保持一致。其他遵守目前项目对 marked 的各项配置,确保 remarkable 里面配置相同。
  3. 测一下 Certain input locks marked markedjs/marked#497 这个问题还存不存在;测一下楼主报的 xss 问题
  4. 前端后端都分别用到了 marked,都要替换
  5. 编辑器的预览功能要保持正常

@alsotang
Copy link
Member

alsotang commented Nov 2, 2014

换好了,接下来要进入过渡阶段了,marked 跟 remarkable 的规则应该会有少许差异。

@fengmk2
Copy link
Member

fengmk2 commented Jan 4, 2015

也会出现性能问题 remarkable

cnpm/cnpmjs.org#552

我现在能随意将 cnode 弄挂,请尽快切换到 markdown-it

@fengmk2
Copy link
Member

fengmk2 commented Jan 4, 2015

@fengmk2
Copy link
Member

fengmk2 commented Jan 4, 2015

html: false https://github.com/cnodejs/nodeclub/blob/master/common/render_helper.js#L23 没有开启 html,那么就没问题了。。。

@alsotang
Copy link
Member

alsotang commented Jan 5, 2015

好的好的,晕呐。

在 2015年1月5日,00:13,fengmk2 <notifications@github.com mailto:notifications@github.com> 写道:

commonmark/commonmark-spec#263 commonmark/commonmark-spec#263

Reply to this email directly or view it on GitHub #440 (comment).

@alsotang
Copy link
Member

alsotang commented Jan 5, 2015

当时之所以禁止 html 也是因为开启了 html 之后,xss 的风险太大,我无法信任 markdown 的库可以把各种 xss 陷阱都绕过。而且就目前的情况来看,只用 markdown 语法大家也能顺畅交流,就直接关闭了。

@fengmk2
Copy link
Member

fengmk2 commented Jan 5, 2015

@alsotang 嗯,将 html 关闭吧。

@fengmk2
Copy link
Member

fengmk2 commented Jan 5, 2015

@alsotang 另外一种选择,开启 html,然后再加一层 js-xss 过滤 https://github.com/leizongmin/js-xss

@alsotang
Copy link
Member

alsotang commented Jan 5, 2015

我当时特意去掉了 js-xss 这个库。我看了他的实现,里面各种正则,也还是属于黑名单的机制。一年前我还绕过了一次:https://cnodejs.org/topic/52b827e9588ff25a72ce204a https://cnodejs.org/topic/52b827e9588ff25a72ce204a

还是不开启 html 了。

在 2015年1月5日,12:59,fengmk2 notifications@github.com 写道:

@alsotang https://github.com/alsotang 另外一种选择,开启 html,然后再加一层 js-xss 过滤 https://github.com/leizongmin/js-xss https://github.com/leizongmin/js-xss

Reply to this email directly or view it on GitHub #440 (comment).

@fengmk2
Copy link
Member

fengmk2 commented Jan 5, 2015

ping @leizongmin

@fengmk2
Copy link
Member

fengmk2 commented Jan 5, 2015

总得与时俱进啊,老雷也会不断修复 bug 的

fengmk2
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Monday, January 5, 2015 at 1:03 PM, alsotang wrote:

我当时特意去掉了 js-xss 这个库。我看了他的实现,里面各种正则,也还是属于黑名单的机制。一年前我还绕过了一次:https://cnodejs.org/topic/52b827e9588ff25a72ce204a https://cnodejs.org/topic/52b827e9588ff25a72ce204a

还是不开启 html 了。

在 2015年1月5日,12:59,fengmk2 <notifications@github.com (mailto:notifications@github.com)> 写道:

@alsotang https://github.com/alsotang 另外一种选择,开启 html,然后再加一层 js-xss 过滤 https://github.com/leizongmin/js-xss https://github.com/leizongmin/js-xss

Reply to this email directly or view it on GitHub #440 (comment).


Reply to this email directly or view it on GitHub (#440 (comment)).

@alsotang
Copy link
Member

alsotang commented Jan 5, 2015

肯定得与时俱进,不过开启 html 的必要性不强吧?

在 2015年1月5日,13:05,fengmk2 notifications@github.com 写道:

总得与时俱进啊,老雷也会不断修复 bug 的

fengmk2
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Monday, January 5, 2015 at 1:03 PM, alsotang wrote:

我当时特意去掉了 js-xss 这个库。我看了他的实现,里面各种正则,也还是属于黑名单的机制。一年前我还绕过了一次:https://cnodejs.org/topic/52b827e9588ff25a72ce204a https://cnodejs.org/topic/52b827e9588ff25a72ce204a

还是不开启 html 了。

在 2015年1月5日,12:59,fengmk2 <notifications@github.com (mailto:notifications@github.com)> 写道:

@alsotang https://github.com/alsotang 另外一种选择,开启 html,然后再加一层 js-xss 过滤 https://github.com/leizongmin/js-xss https://github.com/leizongmin/js-xss

Reply to this email directly or view it on GitHub #440 (comment).


Reply to this email directly or view it on GitHub (#440 (comment)).


Reply to this email directly or view it on GitHub #440 (comment).

@leizongmin
Copy link
Contributor

js-xss是基于白名单过滤的,但是实现过程中总会有点Bug,使用过程中若发现安全问题我会马上修复,不过这需要大家共同参与,提issue。我希望js-xss模块是由大家共同完善的。

@fengmk2
Copy link
Member

fengmk2 commented Jan 6, 2015

现在完全过滤html,导致一些3年前比较好的文章完全无法看了

Sent from my iPhone

On Jan 6, 2015, at 9:21 AM, 老雷 notifications@github.com wrote:

js-xss是基于白名单过滤的,但是实现过程中总会有点Bug,使用过程中若发现安全问题我会马上修复,不过这需要大家共同参与,提issue。我希望js-xss模块是由大家共同完善的。


Reply to this email directly or view it on GitHub.

@leizongmin
Copy link
Contributor

我个人意见是需要开放部分HTML标签,只支持Markdown显得太保守。总不能因为遇到一点问题就一刀切去掉HTML支持。

@fengmk2 fengmk2 reopened this Jan 6, 2015
@fengmk2
Copy link
Member

fengmk2 commented Jan 6, 2015

重新 reopen,开放讨论

@fengmk2
Copy link
Member

fengmk2 commented Jan 6, 2015

@alsotang
Copy link
Member

alsotang commented Jan 6, 2015

有历史遗留问题是肯定的,当然这也导致了原来很多隐藏的 xss 都失效了

在 2015年1月6日,11:24,fengmk2 notifications@github.com 写道:

https://cnodejs.org/topic/4f16442ccae1f4aa270010ab https://cnodejs.org/topic/4f16442ccae1f4aa270010ab

Reply to this email directly or view it on GitHub #440 (comment).

@alsotang
Copy link
Member

alsotang commented Jan 6, 2015

上到 github,下到百度贴吧,谁敢支持用户回复 html

在 2015年1月6日,11:37,alsotang alsotang@gmail.com 写道:

有历史遗留问题是肯定的,当然这也导致了原来很多隐藏的 xss 都失效了

在 2015年1月6日,11:24,fengmk2 <notifications@github.com mailto:notifications@github.com> 写道:

https://cnodejs.org/topic/4f16442ccae1f4aa270010ab https://cnodejs.org/topic/4f16442ccae1f4aa270010ab

Reply to this email directly or view it on GitHub #440 (comment).

@albertshaw
Copy link
Author

白名单支持是可以的吧,markdown其实就是相当于变相白名单嘛。

@fengmk2
Copy link
Member

fengmk2 commented Jan 6, 2015

github支持html的。
百度,为什么要学百度,百度md都不支持

Sent from my iPhone

On Jan 6, 2015, at 11:42 AM, alsotang notifications@github.com wrote:

上到 github,下到百度贴吧,谁敢支持用户回复 html

在 2015年1月6日,11:37,alsotang alsotang@gmail.com 写道:

有历史遗留问题是肯定的,当然这也导致了原来很多隐藏的 xss 都失效了

在 2015年1月6日,11:24,fengmk2 <notifications@github.com mailto:notifications@github.com> 写道:

https://cnodejs.org/topic/4f16442ccae1f4aa270010ab https://cnodejs.org/topic/4f16442ccae1f4aa270010ab

Reply to this email directly or view it on GitHub #440 (comment).


Reply to this email directly or view it on GitHub.

@alsotang
Copy link
Member

alsotang commented Jan 7, 2015

失误失误,github 确实是支持 html 的:https://github.com/github/markup/tree/master#html-sanitization https://github.com/github/markup/tree/master#html-sanitization

在 2015年1月6日,21:03,fengmk2 notifications@github.com 写道:

github支持html的。
百度,为什么要学百度,百度md都不支持

Sent from my iPhone

On Jan 6, 2015, at 11:42 AM, alsotang notifications@github.com wrote:

上到 github,下到百度贴吧,谁敢支持用户回复 html

在 2015年1月6日,11:37,alsotang alsotang@gmail.com 写道:

有历史遗留问题是肯定的,当然这也导致了原来很多隐藏的 xss 都失效了

在 2015年1月6日,11:24,fengmk2 <notifications@github.com mailto:notifications@github.com> 写道:

https://cnodejs.org/topic/4f16442ccae1f4aa270010ab https://cnodejs.org/topic/4f16442ccae1f4aa270010ab

Reply to this email directly or view it on GitHub #440 (comment).


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub #440 (comment).

@alsotang
Copy link
Member

@fengmk2 https://cnodejs.org/topic/4f16442ccae1f4aa270010ab 老的帖子已经可以看了,还在微调样式中

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants