Additional protections for HTTP endpoints

Reject browser-based requests that don't require a CORS preflight
request [1] if they don't come from the connector or include
Zotero-Allowed-Request: 1

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Simple_requests
This commit is contained in:
Dan Stillman 2019-07-15 07:03:32 -04:00
parent 920fb3bd63
commit 2603373b86
3 changed files with 150 additions and 34 deletions

View file

@ -1135,10 +1135,6 @@ Zotero.Server.Connector.Import.prototype = {
permitBookmarklet: false, permitBookmarklet: false,
init: async function (requestData) { init: async function (requestData) {
if (!('X-Zotero-Connector-API-Version' in requestData.headers)) {
return 400;
}
let translate = new Zotero.Translate.Import(); let translate = new Zotero.Translate.Import();
translate.setString(requestData.data); translate.setString(requestData.data);
let translators = await translate.getTranslators(); let translators = await translate.getTranslators();

View file

@ -31,6 +31,7 @@ Zotero.Server = new function() {
204:"No Content", 204:"No Content",
300:"Multiple Choices", 300:"Multiple Choices",
400:"Bad Request", 400:"Bad Request",
403:"Forbidden",
404:"Not Found", 404:"Not Found",
409:"Conflict", 409:"Conflict",
412:"Precondition Failed", 412:"Precondition Failed",
@ -239,35 +240,44 @@ Zotero.Server.DataListener.prototype._headerFinished = function() {
Zotero.debug(this.header, 5); Zotero.debug(this.header, 5);
const methodRe = /^([A-Z]+) ([^ \r\n?]+)(\?[^ \r\n]+)?/; // Parse headers into this.headers with lowercase names
const hostRe = /[\r\n]Host: *(localhost|127\.0\.0\.1)(:[0-9]+)?[\r\n]/i; this.headers = {};
const contentTypeRe = /[\r\n]Content-Type: *([^ \r\n]+)/i; var headerLines = this.header.trim().split(/\r\n/);
for (let line of headerLines) {
const originRe = /[\r\n]Origin: *([^ \r\n]+)/i; line = line.trim();
var m = originRe.exec(this.header); let pos = line.indexOf(':');
if (m) { if (pos == -1) {
this.origin = m[1]; continue;
} }
else { let k = line.substr(0, pos).toLowerCase();
const bookmarkletRe = /[\r\n]Zotero-Bookmarklet: *([^ \r\n]+)/i; let v = line.substr(pos + 1).trim();
var m = bookmarkletRe.exec(this.header); this.headers[k] = v;
if (m) this.origin = "https://www.zotero.org"; }
if (this.headers.origin) {
this.origin = this.headers.origin;
}
else if (this.headers['zotero-bookmarklet']) {
this.origin = "https://www.zotero.org";
} }
if (!Zotero.isServer) { if (!Zotero.isServer) {
// Make sure the Host header is set to localhost/127.0.0.1 to prevent DNS rebinding attacks // Make sure the Host header is set to localhost/127.0.0.1 to prevent DNS rebinding attacks
if (!hostRe.exec(this.header)) { const hostRe = /^(localhost|127\.0\.0\.1)(:[0-9]+)?$/i;
if (!hostRe.test(this.headers.host)) {
this._requestFinished(this._generateResponse(400, "text/plain", "Invalid Host header\n")); this._requestFinished(this._generateResponse(400, "text/plain", "Invalid Host header\n"));
return; return;
} }
} }
// get first line of request // get first line of request
const methodRe = /^([A-Z]+) ([^ \r\n?]+)(\?[^ \r\n]+)?/;
var method = methodRe.exec(this.header); var method = methodRe.exec(this.header);
// get content-type // get content-type
var contentType = contentTypeRe.exec(this.header); var contentType = this.headers['content-type'];
if(contentType) { if (contentType) {
var splitContentType = contentType[1].split(/\s*;/); let splitContentType = contentType.split(/\s*;/);
this.contentType = splitContentType[0]; this.contentType = splitContentType[0];
} }
@ -288,10 +298,10 @@ Zotero.Server.DataListener.prototype._headerFinished = function() {
} else if(method[1] == "GET") { } else if(method[1] == "GET") {
this._processEndpoint("GET", null); // async this._processEndpoint("GET", null); // async
} else if(method[1] == "POST") { } else if(method[1] == "POST") {
const contentLengthRe = /[\r\n]Content-Length: +([0-9]+)/i; const contentLengthRe = /^([0-9]+)$/;
// parse content length // parse content length
var m = contentLengthRe.exec(this.header); var m = contentLengthRe.exec(this.headers['content-length']);
if(!m) { if(!m) {
this._requestFinished(this._generateResponse(400, "text/plain", "Content-length not provided\n")); this._requestFinished(this._generateResponse(400, "text/plain", "Content-length not provided\n"));
return; return;
@ -408,13 +418,46 @@ Zotero.Server.DataListener.prototype._processEndpoint = Zotero.Promise.coroutine
} }
} }
// Reject browser-based requests that don't require a CORS preflight request [1] if they
// don't come from the connector or include Zotero-Allowed-Request
//
// Endpoints that can be triggered with a simple request can be whitelisted if they don't
// trigger any actions
//
// [1] https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Simple_requests
var whitelistedEndpoints = [
'/test/translate/test.html',
'/test/translate/test.pdf',
'/test/translate/does_not_exist.html',
];
var simpleRequestContentTypes = [
'application/x-www-form-urlencoded',
'multipart/form-data',
'text/plain'
];
var isBrowser = this.headers['user-agent'].startsWith('Mozilla/')
// Origin isn't sent via fetch() for HEAD/GET, but for crazy UA strings, protecting
// POST requests is better than nothing
|| 'origin' in this.headers;
if (isBrowser
&& !this.headers['x-zotero-connector-api-version']
&& !this.headers['zotero-allowed-request']
&& (!endpoint.supportedDataTypes
|| endpoint.supportedDataTypes == '*'
|| endpoint.supportedDataTypes.some(type => simpleRequestContentTypes.includes(type)))
&& !whitelistedEndpoints.includes(this.pathname)
&& !(this.contentType && !simpleRequestContentTypes.includes(this.contentType))) {
this._requestFinished(this._generateResponse(403, "text/plain", "Request not allowed\n"));
return;
}
var decodedData = null; var decodedData = null;
if(postData && this.contentType) { if(postData && this.contentType) {
// check that endpoint supports contentType // check that endpoint supports contentType
var supportedDataTypes = endpoint.supportedDataTypes; var supportedDataTypes = endpoint.supportedDataTypes;
if(supportedDataTypes && supportedDataTypes != '*' if(supportedDataTypes && supportedDataTypes != '*'
&& supportedDataTypes.indexOf(this.contentType) === -1) { && supportedDataTypes.indexOf(this.contentType) === -1) {
this._requestFinished(this._generateResponse(400, "text/plain", "Endpoint does not support content-type\n")); this._requestFinished(this._generateResponse(400, "text/plain", "Endpoint does not support content-type\n"));
return; return;
} }

View file

@ -1323,9 +1323,19 @@ describe("Connector Server", function () {
describe('/connector/installStyle', function() { describe('/connector/installStyle', function() {
var endpoint; var endpoint;
var style;
before(function() { before(function() {
endpoint = connectorServerPath + "/connector/installStyle"; endpoint = connectorServerPath + "/connector/installStyle";
style = `<?xml version="1.0" encoding="utf-8"?>
<style xmlns="http://purl.org/net/xbiblio/csl" version="1.0" default-locale="de-DE">
<info>
<title>Test1</title>
<id>http://www.example.com/test2</id>
<link href="http://www.zotero.org/styles/cell" rel="independent-parent"/>
</info>
</style>
`;
}); });
it('should reject styles with invalid text', function* () { it('should reject styles with invalid text', function* () {
@ -1358,15 +1368,6 @@ describe("Connector Server", function () {
}); });
}); });
var style = `<?xml version="1.0" encoding="utf-8"?>
<style xmlns="http://purl.org/net/xbiblio/csl" version="1.0" default-locale="de-DE">
<info>
<title>Test1</title>
<id>http://www.example.com/test2</id>
<link href="http://www.zotero.org/styles/cell" rel="independent-parent"/>
</info>
</style>
`;
var response = yield Zotero.HTTP.request( var response = yield Zotero.HTTP.request(
'POST', 'POST',
endpoint, endpoint,
@ -1379,6 +1380,68 @@ describe("Connector Server", function () {
assert.equal(response.response, JSON.stringify({name: 'Test1'})); assert.equal(response.response, JSON.stringify({name: 'Test1'}));
Zotero.Styles.install.restore(); Zotero.Styles.install.restore();
}); });
it('should accept text/plain request with X-Zotero-Connector-API-Version or Zotero-Allowed-Request', async function () {
sinon.stub(Zotero.Styles, 'install').callsFake(function(style) {
var parser = Components.classes["@mozilla.org/xmlextras/domparser;1"]
.createInstance(Components.interfaces.nsIDOMParser),
doc = parser.parseFromString(style, "application/xml");
return Zotero.Promise.resolve({
styleTitle: Zotero.Utilities.xpathText(
doc, '/csl:style/csl:info[1]/csl:title[1]', Zotero.Styles.ns
),
styleID: Zotero.Utilities.xpathText(
doc, '/csl:style/csl:info[1]/csl:id[1]', Zotero.Styles.ns
)
});
});
// X-Zotero-Connector-API-Version
var response = await Zotero.HTTP.request(
'POST',
endpoint,
{
headers: {
"Content-Type": "text/plain",
"X-Zotero-Connector-API-Version": "2"
},
body: style
}
);
assert.equal(response.status, 201);
// Zotero-Allowed-Request
response = await Zotero.HTTP.request(
'POST',
endpoint,
{
headers: {
"Content-Type": "text/plain",
"Zotero-Allowed-Request": "1"
},
body: style
}
);
assert.equal(response.status, 201);
Zotero.Styles.install.restore();
});
it('should reject text/plain request without X-Zotero-Connector-API-Version', async function () {
var req = await Zotero.HTTP.request(
'POST',
endpoint,
{
headers: {
"Content-Type": "text/plain"
},
body: style,
successCodes: [403]
}
);
assert.equal(req.status, 403);
});
}); });
describe('/connector/import', function() { describe('/connector/import', function() {
@ -1404,6 +1467,20 @@ describe("Connector Server", function () {
assert.equal(error.xmlhttp.status, 400); assert.equal(error.xmlhttp.status, 400);
}); });
it('should reject requests without X-Zotero-Connector-API-Version', async function () {
var req = await Zotero.HTTP.request(
'POST',
endpoint,
{
headers: {
"Content-Type": "text/plain"
},
successCodes: [403]
}
);
assert.equal(req.status, 403);
});
it('should import resources (BibTeX) into selected collection', function* () { it('should import resources (BibTeX) into selected collection', function* () {
var collection = yield createDataObject('collection'); var collection = yield createDataObject('collection');
yield waitForItemsLoad(win); yield waitForItemsLoad(win);