-
Notifications
You must be signed in to change notification settings - Fork 26
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
enhance: other URL schemes #73
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #73 +/- ##
========================================
Coverage 92.27% 92.27%
========================================
Files 4 4
Lines 531 531
Branches 67 67
========================================
Hits 490 490
Misses 41 41 Continue to review full report at Codecov.
|
🙏 |
wait, I think, after this pull-request merged, it can XSS by like |
😨 You are right. |
I don't know, but any URL schemes has a possibility dangerous in the future or specific environment. I prefer allow-list because deny-list may be bypassed if implementation is vulnerable for hacky escape or other something. I think "got many 'please add XXXX protocol' issue" is better than possibility vulnerable. |
Hmm, or maybe a content security policy could avoid such things. <meta http-equiv="Content-Security-Policy" content="script-src 'self';"> This would allow JavaScript from |
😕 If the browser is vulnerable, I don't think it would be MFM's or Misskey's responsibility to fix it? |
yeah, If library users are using CSP correctly, XSS are never executed. but we shouldn't expect that 100% library users are using CSP. actually I think 99% of users are misskey-dev/misskey, but some clients can using this library too. and imagine what occured if this library used in Electron apps without CSP and user clicked javascript link... |
"implementation" of |
セキュリティの側面から見てなかったので、助かった。 また、ユーザーアプリ側からホワイトリストをカスタマイズ可能にすることもできそう。(許可するかどうかは要検討) このPRの内容からは若干反れるので、別のissueとした方が良さそう。 |
うん、私もセキュリティの観点はすっぽり抜けてた |
This reverts commit a731592.
In the future, we may allow schemas other than http/https in a whitelist or blacklist format, but for now, we need to revert for safety. 🙏🏻 |
resolves #72
👀 CHANGELOG.md ?
npm run test
✅npm run lint
✅npm run api
✅