alibaba / rax

🐰 Rax is a progressive framework for building universal application. https://rax.js.org
Other
8k stars 627 forks source link

fix: prevent xss in rax-document #2273

Closed fengzilong closed 2 years ago

fengzilong commented 2 years ago

closes #2272

这个 PR 也可以提到 rax-app 那边,因为 __initialDatarax-app 生成的字符串,但是让开发者升级 rax-app 的成本比升级 rax-document 大,所以选择在这边提 PR

SoloJiang commented 2 years ago
  1. 这部分 xss 的风险是不是让开发者处理比较好,直接替换目标字符可能存在不符合预期的情况(并且存在 break change 的可能)?
  2. 即使处理,确实应该在 rax-app 拿到 initialData 的时候处理,因为不止有 src/document 的时候可能存在这个问题,默认无 document 的时候也存在相应的问题
fengzilong commented 2 years ago
  1. 这部分 xss 的风险是不是让开发者处理比较好,直接替换目标字符可能存在不符合预期的情况(并且存在 break change 的可能)?

使用 unicode 替换,还是原本的字符,<>\ 在 json 中理论上只会出现在字符串里面,而字符串替换前后理论上应该是相等的

比如

'<' === '\u003C'
'>' === '\u003E'
'/' === '\u002F'

一般有几种使用场景:

感觉不会有什么问题,可以在这几种场景再验证下...

如果是 break 的问题,是不是可以额外配置一个属性,让用户手动开启防 XSS

不过让开发者自己处理的话也可以,但是最好能够出一个官方的 npm 包,因为在 getInitialProps 里面处理 XSS 还挺麻烦的,嵌套层数很深的对象,需要遍历查找字符串类型的 value,再做 XSS 处理

  1. 即使处理,确实应该在 rax-app 拿到 initialData 的时候处理,因为不止有 src/document 的时候可能存在这个问题,默认无 document 的时候也存在相应的问题

这个我可以去 rax-app 那边提下 PR,这个可以先关掉

SoloJiang commented 2 years ago

我觉得框架在这方面还是尽量做减法,这个应该是一个类似 lodash 的工具包,因为框架视角来看开发者的每次渲染输入都可能是危险的,而社区内的普遍做法是交由开发者自己去处理

fengzilong commented 2 years ago

我觉得框架在这方面还是尽量做减法

这部分改动对用户是无感知的,感觉问题不大,有些框架也会把 "更安全" 作为 feature

这个应该是一个类似 lodash 的工具包,因为框架视角来看开发者的每次渲染输入都可能是危险的

我觉得 rax-document 这个不能算简单的工具包了,已经耦合了很多 rax-plugin-ssr 的特定逻辑,属于 rax ssr 生态里面的一部分了(如果后面准备弃用或者不推荐 rax-document 的话,那就是另一回事了...我可以提 PR 到 rax-plugin-ssr 那边)

而社区内的普遍做法是交由开发者自己去处理

https://github.com/vercel/next.js/blob/5760ceac8668af23b81e66026259385b33159e03/packages/next/pages/_document.tsx#L804

我刚看了下 Next.js 的实现,他们是做了这部分处理的,这个 PR 是不是也可以考虑下呢

SoloJiang commented 2 years ago

放在 plugin-ssr 统一处理吧

fengzilong commented 2 years ago

放在 plugin-ssr 统一处理吧

👌

fengzilong commented 2 years ago

新的 PR 地址:https://github.com/raxjs/rax-app/pull/856