improve URL parsing

uses `new URL()` and `decodeURI()` instead of `url.parse()`
also checks that the requested file is in a subdirectory of `public/` before serving the file

fixes path traversal vulnerability GHSA-5cxq-25mp-q5f2
This commit is contained in:
jomo 2024-02-01 22:24:29 +01:00
parent 9cb32a843f
commit bba004acc7
6 changed files with 79 additions and 60 deletions

View File

@ -14,12 +14,10 @@ function handle_default(img_status, userId, size, def, req, err, callback) {
if (defname !== "steve" && defname !== "mhf_steve" && defname !== "alex" && defname !== "mhf_alex") {
if (helpers.id_valid(def)) {
// clean up the old URL to match new image
var parsed = req.url;
delete parsed.query.default;
delete parsed.search;
parsed.path_list[1] = def;
parsed.pathname = "/" + parsed.path_list.join("/");
var newUrl = url.format(parsed);
req.url.searchParams.delete('default');
req.url.path_list[1] = def;
req.url.pathname = req.url.path_list.join('/');
var newUrl = req.url.toString();
callback({
status: img_status,
redirect: newUrl,
@ -53,9 +51,9 @@ function handle_default(img_status, userId, size, def, req, err, callback) {
// GET avatar request
module.exports = function(req, callback) {
var userId = (req.url.path_list[1] || "").split(".")[0];
var size = parseInt(req.url.query.size) || config.avatars.default_size;
var def = req.url.query.default;
var overlay = Object.prototype.hasOwnProperty.call(req.url.query, "overlay") || Object.prototype.hasOwnProperty.call(req.url.query, "helm");
var size = parseInt(req.url.searchParams.get("size")) || config.avatars.default_size;
var def = req.url.searchParams.get("default");
var overlay = req.url.searchParams.has("overlay") || req.url.searchParams.has("helm");
// check for extra paths
if (req.url.path_list.length > 2) {

View File

@ -4,7 +4,7 @@ var cache = require("../cache");
// GET cape request
module.exports = function(req, callback) {
var userId = (req.url.path_list[1] || "").split(".")[0];
var def = req.url.query.default;
var def = req.url.searchParams.get('default');
var rid = req.id;
// check for extra paths

View File

@ -17,12 +17,10 @@ function handle_default(rid, scale, overlay, body, img_status, userId, size, def
if (defname !== "steve" && defname !== "mhf_steve" && defname !== "alex" && defname !== "mhf_alex") {
if (helpers.id_valid(def)) {
// clean up the old URL to match new image
var parsed = req.url;
delete parsed.query.default;
delete parsed.search;
parsed.path_list[2] = def;
parsed.pathname = "/" + parsed.path_list.join("/");
var newUrl = url.format(parsed);
req.url.searchParams.delete('default');
req.url.path_list[2] = def;
req.url.pathname = req.url.path_list.join('/');
var newUrl = req.url.toString();
callback({
status: img_status,
redirect: newUrl,
@ -62,9 +60,9 @@ module.exports = function(req, callback) {
var rid = req.id;
var body = raw_type === "body";
var userId = (req.url.path_list[2] || "").split(".")[0];
var def = req.url.query.default;
var scale = parseInt(req.url.query.scale) || config.renders.default_scale;
var overlay = Object.prototype.hasOwnProperty.call(req.url.query, "overlay") || Object.prototype.hasOwnProperty.call(req.url.query, "helm");
var def = req.url.searchParams.get("default");
var scale = parseInt(req.url.searchParams.get("scale")) || config.renders.default_scale;
var overlay = req.url.searchParams.has("overlay") || req.url.searchParams.has("helm");
// check for extra paths
if (req.url.path_list.length > 3) {

View File

@ -14,12 +14,10 @@ function handle_default(img_status, userId, def, req, err, callback) {
if (defname !== "steve" && defname !== "mhf_steve" && defname !== "alex" && defname !== "mhf_alex") {
if (helpers.id_valid(def)) {
// clean up the old URL to match new image
var parsed = req.url;
delete parsed.query.default;
delete parsed.search;
parsed.path_list[1] = def;
parsed.pathname = "/" + parsed.path_list.join("/");
var newUrl = url.format(parsed);
req.url.searchParams.delete('default');
req.url.path_list[1] = def;
req.url.pathname = req.url.path_list.join('/');
var newUrl = req.url.toString();
callback({
status: img_status,
redirect: newUrl,
@ -62,7 +60,7 @@ function handle_default(img_status, userId, def, req, err, callback) {
// GET skin request
module.exports = function(req, callback) {
var userId = (req.url.path_list[1] || "").split(".")[0];
var def = req.url.query.default;
var def = req.url.searchParams.get("default");
var rid = req.id;
// check for extra paths

View File

@ -22,24 +22,33 @@ var routes = {
// serves assets from lib/public
function asset_request(req, callback) {
var filename = path.join(__dirname, "public", req.url.path_list.join("/"));
fs.access(filename, function(fs_err) {
if (!fs_err) {
fs.readFile(filename, function(err, data) {
callback({
body: data,
type: mime.getType(filename),
err: err,
const filename = path.join(__dirname, "public", ...req.url.path_list);
const relative = path.relative(path.join(__dirname, "public"), filename);
if (relative && !relative.startsWith('..') && !path.isAbsolute(relative)) {
fs.access(filename, function(fs_err) {
if (!fs_err) {
fs.readFile(filename, function(err, data) {
callback({
body: data,
type: mime.getType(filename),
err: err,
});
});
});
} else {
callback({
body: "Not found",
status: -2,
code: 404,
});
}
});
} else {
callback({
body: "Not found",
status: -2,
code: 404,
});
}
});
} else {
callback({
body: "Forbidden",
status: -2,
code: 403,
});
}
}
// generates a 12 character random string
@ -47,26 +56,18 @@ function request_id() {
return Math.random().toString(36).substring(2, 14);
}
// splits a URL path into an Array
// the path is resolved and decoded
// splits decoded URL path into an Array
function path_list(pathname) {
// remove double and trailing slashes
pathname = pathname.replace(/\/\/+/g, "/").replace(/(.)\/$/, "$1");
var list = pathname.split("/");
list.shift();
for (var i = 0; i < list.length; i++) {
// URL decode
list[i] = querystring.unescape(list[i]);
}
return list;
}
// handles the +req+ by routing to the request to the appropriate module
function requestHandler(req, res) {
req.url = url.parse(req.url, true);
req.url.query = req.url.query || {};
req.url = new URL(decodeURI(req.url), 'http://' + req.headers.host);
req.url.pathname = path.resolve('/', req.url.pathname);
req.url.path_list = path_list(req.url.pathname);
req.id = request_id();
req.start = Date.now();

View File

@ -315,7 +315,7 @@ describe("Crafatar", function() {
"avatar with non-existent uuid defaulting to uuid": {
url: "http://localhost:3000/avatars/00000000000000000000000000000000?size=16&default=853c80ef3c3749fdaa49938b674adae6",
crc32: [0],
redirect: "/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
redirect: "http://localhost:3000/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
},
"avatar with non-existent uuid defaulting to url": {
url: "http://localhost:3000/avatars/00000000000000000000000000000000?size=16&default=http%3A%2F%2Fexample.com%2FCaseSensitive",
@ -337,7 +337,7 @@ describe("Crafatar", function() {
"overlay avatar with non-existent uuid defaulting to uuid": {
url: "http://localhost:3000/avatars/00000000000000000000000000000000?size=16&default=853c80ef3c3749fdaa49938b674adae6",
crc32: [0],
redirect: "/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
redirect: "http://localhost:3000/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
},
"overlay avatar with non-existent uuid defaulting to url": {
url: "http://localhost:3000/avatars/00000000000000000000000000000000?size=16&overlay&default=http%3A%2F%2Fexample.com%2FCaseSensitive",
@ -372,7 +372,7 @@ describe("Crafatar", function() {
"skin with non-existent uuid defaulting to uuid": {
url: "http://localhost:3000/skins/00000000000000000000000000000000?size=16&default=853c80ef3c3749fdaa49938b674adae6",
crc32: [0],
redirect: "/skins/853c80ef3c3749fdaa49938b674adae6?size=16",
redirect: "http://localhost:3000/skins/853c80ef3c3749fdaa49938b674adae6?size=16",
},
"skin with non-existent uuid defaulting to url": {
url: "http://localhost:3000/skins/00000000000000000000000000000000?default=http%3A%2F%2Fexample.com%2FCaseSensitive",
@ -394,7 +394,7 @@ describe("Crafatar", function() {
"head render with non-existent uuid defaulting to uuid": {
url: "http://localhost:3000/renders/head/00000000000000000000000000000000?scale=2&default=853c80ef3c3749fdaa49938b674adae6",
crc32: [0],
redirect: "/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2",
redirect: "http://localhost:3000/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2",
},
"head render with non-existent uuid defaulting to url": {
url: "http://localhost:3000/renders/head/00000000000000000000000000000000?scale=2&default=http%3A%2F%2Fexample.com%2FCaseSensitive",
@ -416,7 +416,7 @@ describe("Crafatar", function() {
"overlay head with non-existent uuid defaulting to uuid": {
url: "http://localhost:3000/renders/head/00000000000000000000000000000000?scale=2&overlay&default=853c80ef3c3749fdaa49938b674adae6",
crc32: [0],
redirect: "/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2&overlay=",
redirect: "http://localhost:3000/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2&overlay=",
},
"overlay head render with non-existent uuid defaulting to url": {
url: "http://localhost:3000/renders/head/00000000000000000000000000000000?scale=2&overlay&default=http%3A%2F%2Fexample.com%2FCaseSensitive",
@ -438,7 +438,7 @@ describe("Crafatar", function() {
"body render with non-existent uuid defaulting to uuid": {
url: "http://localhost:3000/renders/body/00000000000000000000000000000000?scale=2&default=853c80ef3c3749fdaa49938b674adae6",
crc32: [0],
redirect: "/renders/body/853c80ef3c3749fdaa49938b674adae6?scale=2",
redirect: "http://localhost:3000/renders/body/853c80ef3c3749fdaa49938b674adae6?scale=2",
},
"body render with non-existent uuid defaulting to url": {
url: "http://localhost:3000/renders/body/00000000000000000000000000000000?scale=2&default=http%3A%2F%2Fexample.com%2FCaseSensitive",
@ -568,6 +568,30 @@ describe("Crafatar", function() {
});
}(loc));
}
it("should return /public resources", function(done) {
request.get("http://localhost:3000/javascript/crafatar.js", function(error, res, body) {
assert.ifError(error);
assert.strictEqual(res.statusCode, 200);
done();
});
});
it("should not allow path traversal on /public", function(done) {
request.get("http://localhost:3000/../server.js", function(error, res, body) {
assert.ifError(error);
assert.strictEqual(res.statusCode, 404);
done();
});
});
it("should not allow encoded path traversal on /public", function(done) {
request.get("http://localhost:3000/%2E%2E/server.js", function(error, res, body) {
assert.ifError(error);
assert.strictEqual(res.statusCode, 404);
done();
});
});
});
// we have to make sure that we test both a 32x64 and 64x64 skin