CVE-2024-24756: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Crafatar serves Minecraft avatars based on the skin for use in external applications. Files outside of the `lib/public/` directory can be requested from the server. Instances running behind Cloudflare (including crafatar.com) are not affected. Instances using the Docker container as shown in the README are affected, but only files within the container can be read. By default, all of the files within the container can also be found in this repository and are not confidential. This vulnerability is patched in 2.1.5.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.5
Published: 2024-02-01T23:15Z
Repository: https://github.com/crafatar/crafatar
Language: JavaScript
Files changed: lib/routes/avatars.js, lib/routes/capes.js, lib/routes/renders.js, lib/routes/skins.js, lib/server.js, test/test.js
Commit date: 2024-02-01 22:24:29 +0100
Commit message: 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

Diff: diff --git a/lib/routes/avatars.js b/lib/routes/avatars.js
index fe6ab70..be8e040 100644
--- a/lib/routes/avatars.js
+++ b/lib/routes/avatars.js
@@ -12,16 +12,14 @@ function handle_default(img_status, userId, size, def, req, err, callback) {
   def = def || skins.default_skin(userId);
   var defname = def.toLowerCase();
   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,
         err: err,
       });
@@ -51,13 +49,13 @@ 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) {
     callback({
       status: -2,
diff --git a/lib/routes/capes.js b/lib/routes/capes.js
index fe38ca9..fc5682a 100644
--- a/lib/routes/capes.js
+++ b/lib/routes/capes.js
@@ -2,11 +2,11 @@ var helpers = require("../helpers");
 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
   if (req.url.path_list.length > 2) {
     callback({
diff --git a/lib/routes/renders.js b/lib/routes/renders.js
index 5a335fe..235a244 100644
--- a/lib/routes/renders.js
+++ b/lib/routes/renders.js
@@ -15,16 +15,14 @@ function handle_default(rid, scale, overlay, body, img_status, userId, size, def
   def = def || skins.default_skin(userId);
   var defname = def.toLowerCase();
   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,
         err: err
       });
@@ -60,13 +58,13 @@ function handle_default(rid, scale, overlay, body, img_status, userId, size, def
 module.exports = function(req, callback) {
   var raw_type = req.url.path_list[1] || "";
   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) {
     callback({
       status: -2,
diff --git a/lib/routes/skins.js b/lib/routes/skins.js
index 6951f46..50e458e 100644
--- a/lib/routes/skins.js
+++ b/lib/routes/skins.js
@@ -12,16 +12,14 @@ function handle_default(img_status, userId, def, req, err, callback) {
   def = def || skins.default_skin(userId);
   var defname = def.toLowerCase();
   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,
         err: err
       });
@@ -60,11 +58,11 @@ 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
   if (req.url.path_list.length > 2) {
     callback({
diff --git a/lib/server.js b/lib/server.js
index ff5981c..6fab129 100644
--- a/lib/server.js
+++ b/lib/server.js
@@ -20,55 +20,56 @@ var routes = {
   capes: require("./routes/capes"),
 };
 
 // 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) {
+  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: data,
-          type: mime.getType(filename),
-          err: err,
+          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
 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();
 
   var local_path = req.url.path_list[0];
   logging.debug(req.id, req.method, req.url.href);
diff --git a/test/test.js b/test/test.js
index 4e9c330..ee00d96 100644
--- a/test/test.js
+++ b/test/test.js
@@ -313,11 +313,11 @@ describe("Crafatar", function() {
         crc32: [73899130],
       },
       "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",
         crc32: [0],
         redirect: "http://example.com/CaseSensitive",
@@ -335,11 +335,11 @@ describe("Crafatar", function() {
         crc32: [73899130],
       },
       "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",
         crc32: [0],
         redirect: "http://example.com/CaseSensitive",
@@ -370,11 +370,11 @@ describe("Crafatar", function() {
         crc32: [427506205],
       },
       "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",
         crc32: [0],
         redirect: "http://example.com/CaseSensitive",
@@ -392,11 +392,11 @@ describe("Crafatar", function() {
         crc32: [4027858557],
       },
       "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",
         crc32: [0],
         redirect: "http://example.com/CaseSensitive",
@@ -414,11 +414,11 @@ describe("Crafatar", function() {
         crc32: [4027858557],
       },
       "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",
         crc32: [0],
         redirect: "http://example.com/CaseSensitive",
@@ -436,11 +436,11 @@ describe("Crafatar", function() {
         crc32: [4280894468],
       },
       "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",
         crc32: [0],
         redirect: "http://example.com/CaseSensitive",
@@ -566,10 +566,34 @@ describe("Crafatar", function() {
             done();
           });
         });
       }(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
   describe("Networking: Render", function() {
     it("should not fail (uuid, 32x64 skin)", function(done) {

Vulnerable code:       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);
  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 def = req.url.query.default;
      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);
  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 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);
  var def = req.url.query.default;
  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) {
          body: data,
          type: mime.getType(filename),
          err: err,
      });
    } else {
      callback({
        body: "Not found",
        status: -2,
        code: 404,
      });
    }
  });
// splits a URL path into an Array
// the path is resolved and decoded
  // remove double and trailing slashes
  pathname = pathname.replace(/\/\/+/g, "/").replace(/(.)\/$/, "$1");
  for (var i = 0; i < list.length; i++) {
    // URL decode
    list[i] = querystring.unescape(list[i]);
  }
  req.url = url.parse(req.url, true);
  req.url.query = req.url.query || {};

        redirect: "/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
        redirect: "/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
        redirect: "/skins/853c80ef3c3749fdaa49938b674adae6?size=16",
        redirect: "/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2",
        redirect: "/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2&overlay=",
        redirect: "/renders/body/853c80ef3c3749fdaa49938b674adae6?scale=2",
Fixed code:       req.url.searchParams.delete('default');
      req.url.path_list[1] = def;
      req.url.pathname = req.url.path_list.join('/');
      var newUrl = req.url.toString();
  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");
  var def = req.url.searchParams.get('default');
      req.url.searchParams.delete('default');
      req.url.path_list[2] = def;
      req.url.pathname = req.url.path_list.join('/');
      var newUrl = req.url.toString();
  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");
      req.url.searchParams.delete('default');
      req.url.path_list[1] = def;
      req.url.pathname = req.url.path_list.join('/');
      var newUrl = req.url.toString();
  var def = req.url.searchParams.get("default");
  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 {
          body: "Not found",
          status: -2,
          code: 404,
      }
    });
  } else {
    callback({
      body: "Forbidden",
      status: -2,
      code: 403,
    });
  }
// splits decoded URL path into an Array
  req.url = new URL(decodeURI(req.url), 'http://' + req.headers.host);
  req.url.pathname = path.resolve('/', req.url.pathname);
        redirect: "http://localhost:3000/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
        redirect: "http://localhost:3000/avatars/853c80ef3c3749fdaa49938b674adae6?size=16",
        redirect: "http://localhost:3000/skins/853c80ef3c3749fdaa49938b674adae6?size=16",
        redirect: "http://localhost:3000/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2",
        redirect: "http://localhost:3000/renders/head/853c80ef3c3749fdaa49938b674adae6?scale=2&overlay=",
        redirect: "http://localhost:3000/renders/body/853c80ef3c3749fdaa49938b674adae6?scale=2",

    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();
      });
    });