rooch-network / rooch

VApp Container with Move Language for Bitcoin ecosystem
https://rooch.network
Apache License 2.0
162 stars 85 forks source link

[portal] add csp rule #2583

Closed newraina closed 2 months ago

newraina commented 2 months ago

Summary

Related with #1958

Add strict CSP rules to the Rooch Portal to prevent potential XSS attacks.

Disadvantage: it will cause all pages to be SSR. But this should be acceptable.

Test Result

Attempting to load script from a third-party domain

Example:

<Script src="https://unpkg.com/react@16.7.0/umd/react.production.min.js" />

The application will refuse the connection:

Refused to load the script 'https://unpkg.com/react@16.7.0/umd/react.production.min.js' because it violates the following Content Security Policy directive: "script-src 'self' 'unsafe-eval' 'unsafe-inline'". Note that 'script-src-elem' was not explicitly set, so 'script-src' is used as a fallback.

Attempting to use inline script

Example:

<button onclick="alert('you are hacked')" />

The application will refuse to exec:

Refused to execute inline event handler because it violates the following Content Security Policy directive: "script-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution. Note that hashes do not apply to event handlers, style attributes and javascript: navigations unless the 'unsafe-hashes' keyword is present.

Attempting to use eval to execute concatenated code

Example:

eval(location.hash.substring(1))

The application will refuse to exec:

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self'".

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rooch-portal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 9, 2024 2:53am
rooch-portal-v2.1 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 9, 2024 2:53am
1 Skipped Deployment | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **rooch** | ⬜️ Ignored ([Inspect](https://vercel.com/rooch/rooch/Hu7JkMgiBBeZEViTd4dSqwcD7eVE)) | [Visit Preview](https://rooch-git-fork-newraina-csp-rule-rooch.vercel.app) | | Sep 9, 2024 2:53am |
wow-sven commented 2 months ago

@jojoo-eth it will cause all pages to be SSR, whether it is affected

wow-sven commented 2 months ago

@newraina 全部ssr,可能会对浏览器钱包相关功能产生影响。目前ui 没有完整的自动测试,需要你验证一下功能

jojoo-eth commented 2 months ago

@newraina 全部ssr,可能会对浏览器钱包相关功能产生影响。目前ui 没有完整的自动测试,需要你验证一下功能

@newraina yes, this could potentially lead to compatibility issues with certain wallet kits or wallets, particularly regarding behaviors such as operations involving local storage.

jojoo-eth commented 2 months ago

and thanks this pr, will have a detail check in the coming days

newraina commented 2 months ago

@newraina 全部ssr,可能会对浏览器钱包相关功能产生影响。目前ui 没有完整的自动测试,需要你验证一下功能

@wow-sven @jojoo-eth

对钱包功能没有影响,所有 use client 的组件都还是浏览器端渲染的。我在本地也用 next build + next start 测试过几个钱包功能,没发现啥问题。

之前可能没说清楚,全部页面 SSR 的意思是原本有些页面在 build 阶段就可以确定内容了(比如 not-found 页面),但现在因为每次请求页面 html 的时候需要一个新的随机字符串,所以每一个页面都会在请求的时候在 server 端重新 render 一次来确定内容。但这并不意味着所有的组件也都会在 server 端 render,原来是 client 组件现在还是。

下面这张图就是这个 PR 导致的 build 结果的前后变化,原本有些显示 ○ (static) 的路由现在也是 ƒ (dynamic)了,除了对页面的请求速度可能有些微不足道的影响外,可以说没有其他任何变化

CleanShot 2024-09-08 at 11 18 18@2x

The wallet feature works just fine, and any component that relies on the client side for rendering hasn't changed a bit.

Might've not made it clear before, but when I said all pages are going through SSR, it meant that certain pages had their content locked in at the build stage (take the not-found page, for example). But now, since every page load demands a fresh random string, each page gets a server-side make to figure out what it should show. This doesn't mean we're moving all components to server-side rendering, though. If it was a client-side component before, it still is.

The pic above lays out the before and after of the build results because of this PR. Routes that used to be marked with a ○ (meaning static) are now tagged with a ƒ (indicating dynamic). Other than a possible slight delay in how fast pages load, there's really nothing else worth mentioning.