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
Fix http server XSS #1714
Merged
+48
−10
Merged
Fix http server XSS #1714
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
Loading status checks…
Fix http server XSS
Low risk xss. If the torrent contains a specially crafted title or file name, and the user starts the WebTorrent HTTP server via createServer(), and then the user visits the HTTP server index page (which lists the contents of the torrent), then the attacker can run JavaScript in this browser context. The reason this seems relatively low risk is that the WebTorrent HTTP server only allows fetching data pieces from the torrent. It doesn't support any other control of the torrent client. So, attacker code could e.g. figure out what content the user is downloading and exfiltrate that to an external domain. This commit mitigates the issue in two ways (either of which could have prevented this XSS on its own): 1. HTML-escape untrusted torrent metadata (name, path, file names, etc.) 2. Add the strictest possible CSP to prevent all connections, scripts, styles, plugins, frames. Every capability is denied.
- Loading branch information
| @@ -1,4 +1,5 @@ | ||
| const arrayRemove = require('unordered-array-remove') | ||
| const escapeHtml = require('escape-html') | ||
| const http = require('http') | ||
| const mime = require('mime') | ||
| const pump = require('pump') | ||
| @@ -90,6 +91,9 @@ function Server (torrent, opts = {}) { | ||
| // Prevent browser mime-type sniffing | ||
| res.setHeader('X-Content-Type-Options', 'nosniff') | ||
|
|
||
| // Defense-in-depth: Set a strict Content Security Policy to mitigate XSS | ||
| res.setHeader('Content-Security-Policy', "base-uri 'none'; default-src 'none'; frame-ancestors 'none'; object-src 'none';") | ||
|
This conversation was marked as resolved
by feross
diracdeltas
Contributor
|
||
|
|
||
| // Allow CORS requests to specify arbitrary headers, e.g. 'Range', | ||
| // by responding to the OPTIONS preflight request with the specified | ||
| // origin and requested headers. | ||
| @@ -147,11 +151,26 @@ function Server (torrent, opts = {}) { | ||
| res.statusCode = 200 | ||
| res.setHeader('Content-Type', 'text/html') | ||
|
|
||
| const listHtml = torrent.files.map((file, i) => `<li><a download="${file.name}" href="/${i}/${file.name}">${file.path}</a> (${file.length} bytes)</li>`).join('<br>') | ||
| const listHtml = torrent.files | ||
| .map((file, i) => ( | ||
| `<li> | ||
| <a | ||
| download="${escapeHtml(file.name)}" | ||
| href="/${escapeHtml(i)}/${escapeHtml(file.name)}" | ||
| > | ||
| ${escapeHtml(file.path)} | ||
| </a> | ||
| (${escapeHtml(file.length)} bytes) | ||
| </li>` | ||
| )) | ||
| .join('<br>') | ||
|
|
||
| const html = getPageHTML( | ||
| `${torrent.name} - WebTorrent`, | ||
| `<h1>${torrent.name}</h1><ol>${listHtml}</ol>` | ||
| `${escapeHtml(torrent.name)} - WebTorrent`, | ||
| ` | ||
| <h1>${escapeHtml(torrent.name)}</h1> | ||
| <ol>${listHtml}</ol> | ||
| ` | ||
| ) | ||
| res.end(html) | ||
| } | ||
| @@ -160,7 +179,10 @@ function Server (torrent, opts = {}) { | ||
| res.statusCode = 404 | ||
| res.setHeader('Content-Type', 'text/html') | ||
|
|
||
| const html = getPageHTML('404 - Not Found', '<h1>404 - Not Found</h1>') | ||
| const html = getPageHTML( | ||
| '404 - Not Found', | ||
| '<h1>404 - Not Found</h1>' | ||
| ) | ||
| res.end(html) | ||
| } | ||
|
|
||
| @@ -214,16 +236,31 @@ function Server (torrent, opts = {}) { | ||
| function serveMethodNotAllowed () { | ||
| res.statusCode = 405 | ||
| res.setHeader('Content-Type', 'text/html') | ||
| const html = getPageHTML('405 - Method Not Allowed', '<h1>405 - Method Not Allowed</h1>') | ||
| const html = getPageHTML( | ||
| '405 - Method Not Allowed', | ||
| '<h1>405 - Method Not Allowed</h1>' | ||
| ) | ||
| res.end(html) | ||
| } | ||
| } | ||
|
|
||
| return server | ||
| } | ||
|
|
||
| // NOTE: Arguments must already be HTML-escaped | ||
| function getPageHTML (title, pageHtml) { | ||
| return `<!DOCTYPE html><html lang="en"><head><meta charset="utf-8"><title>${title}</title></head><body>${pageHtml}</body></html>` | ||
| return ` | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>${title}</title> | ||
| </head> | ||
| <body> | ||
| ${pageHtml} | ||
| </body> | ||
| </html> | ||
| ` | ||
| } | ||
|
|
||
| // From https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent | ||
ProTip!
Use n and p to navigate between commits in a pull request.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
this is super minor but you could also set https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/form-action to none if you wanted to lock it down more