-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Target restriction #111
base: master
Are you sure you want to change the base?
Target restriction #111
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,6 +244,8 @@ function getHandler(options, proxy) { | |
maxRedirects: 5, // Maximum number of redirects to be followed. | ||
originBlacklist: [], // Requests from these origins will be blocked. | ||
originWhitelist: [], // If non-empty, requests not from an origin in this list will be blocked. | ||
targetBlacklist: [], // Requests to these targets will be blocked. | ||
targetWhitelist: [], // If non-empty, requests not to an target in this list will be blocked. | ||
checkRateLimit: null, // Function that may enforce a rate-limit by returning a non-empty string. | ||
redirectSameOrigin: false, // Redirect the client to the requested URL for same-origin requests. | ||
requireHeader: null, // Require a header to be set? | ||
|
@@ -342,6 +344,19 @@ function getHandler(options, proxy) { | |
return; | ||
} | ||
|
||
var target = location.protocol + '//' + location.hostname; | ||
if (corsAnywhere.targetBlacklist.indexOf(target) >= 0) { | ||
res.writeHead(403, 'Forbidden', cors_headers); | ||
res.end('The target "' + target + '" was blacklisted by the operator of this proxy.'); | ||
return; | ||
} | ||
|
||
if (corsAnywhere.targetWhitelist.length && corsAnywhere.targetWhitelist.indexOf(target) === -1) { | ||
res.writeHead(403, 'Forbidden', cors_headers); | ||
res.end('The target "' + target + '" was not whitelisted by the operator of this proxy.'); | ||
return; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check can be bypassed if the whitelisted target site has an open redirect. A better place to enforce this is in the Locally I started with something like this (many months ago), but I never got to finish the implementation. function proxyRequest(req, res, proxy) {
var location = req.corsAnywhereRequestState.location;
+ // TODO: Add something like this?
+ // For https://github.com/Rob--W/cors-anywhere/issues/67
+ if (req.corsAnywhereRequestState.checkRequestAllowed &&
+ !req.corsAnywhereRequestState.checkRequestAllowed(location)) {
+ res.writeHead(403, 'Forbidden', withCORS({}, req));
+ res.end('The requested resource was blocked by the operator of this proxy.');
+ return;
+ } The idea behind this is that the validator can be as flexible as anyone wants to, with some default implementation in Matching by prefix is the simplest, but is it sufficient for most use cases? I recall another bug that wanted to match by "file extension". |
||
var rateLimitMessage = corsAnywhere.checkRateLimit && corsAnywhere.checkRateLimit(origin); | ||
if (rateLimitMessage) { | ||
res.writeHead(429, 'Too Many Requests', cors_headers); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -548,6 +548,54 @@ describe('originWhitelist', function() { | |
}); | ||
}); | ||
|
||
describe('targetBlacklist', function() { | ||
before(function() { | ||
cors_anywhere = createServer({ | ||
targetBlacklist: ['http://example.com'], | ||
}); | ||
cors_anywhere_port = cors_anywhere.listen(0).address().port; | ||
}); | ||
after(stopServer); | ||
|
||
it('GET /http://example.com with denied target http://example.com', function(done) { | ||
request(cors_anywhere) | ||
.get('/http://example.com/') | ||
.expect('Access-Control-Allow-Origin', '*') | ||
.expect(403, done); | ||
}); | ||
|
||
it('GET /https://example.com with denied target http://example.com', function(done) { | ||
request(cors_anywhere) | ||
.get('/https://example.com/') // Note: different scheme! | ||
.expect('Access-Control-Allow-Origin', '*') | ||
.expect(200, done); | ||
}); | ||
}); | ||
|
||
describe('targetWhitelist', function() { | ||
before(function() { | ||
cors_anywhere = createServer({ | ||
targetWhitelist: ['http://example.com'], | ||
}); | ||
cors_anywhere_port = cors_anywhere.listen(0).address().port; | ||
}); | ||
after(stopServer); | ||
|
||
it('GET /http://example.com with permitted target http://example.com', function(done) { | ||
request(cors_anywhere) | ||
.get('/http://example.com/') | ||
.expect('Access-Control-Allow-Origin', '*') | ||
.expect(200, done); | ||
}); | ||
|
||
it('GET /https://example.com with permitted target http://example.com', function(done) { | ||
request(cors_anywhere) | ||
.get('/https://example.com') // Note: different scheme! | ||
.expect('Access-Control-Allow-Origin', '*') | ||
.expect(403, done); | ||
}); | ||
}); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add tests where the request to a whitelisted target is redirected. One test for redirection to a whitelisted target, and another test for redirection to a blacklisted target. |
||
describe('checkRateLimit', function() { | ||
afterEach(stopServer); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please undo the removal of every two spaces at the end (
). These are necessary to force a line break. After your line the
Example: ...
is not on a separate line any more.