fix(controller): sendFile honours absolute directory outside the web root

changelog.d/3077-sendfile-absolute-directory.fixed.md

@@ -0,0 +1 @@
+- `sendFile()`: an absolute `directory` argument is now used verbatim instead of being post-processed, honouring the documented "must be a full path … outside of the web root" contract. Previously the resolver ran the caller-supplied absolute path through the `/wheels` mapping rewrite — substring-hijacking any directory containing `/wheels` (e.g. `/var/www/wheels/uploads`) on every engine — and through an `ExpandPath()` fallback that web-root-prefixed the path on Adobe CF, so serving files from outside the web root threw `Wheels.FileNotFound` there. Relative `filePath`-based resolution and the `..`-traversal guard are unchanged (#3077)

vendor/wheels/controller/miscellaneous.cfc

@@ -260,34 +260,58 @@ component {
 			if (!Len(local.folder)) {
 				local.folder = local.relativeRoot & $get("filePath");
 			}
-			if (Left(local.folder, Len(local.root)) == local.root) {
-				local.folder = RemoveChars(local.folder, 1, Len(local.root));
-			}
-			local.fullPath = Replace(local.folder, "\", "/", "all");
-			local.fullPath = ListAppend(local.fullPath, arguments.file, "/");
-			// https://github.com/wheels-dev/wheels/issues/873 Don't expand path if already contains root
-			if (local.fullPath DOES NOT CONTAIN Replace(local.root, "\", "/", "all")) {
-				//added this section for the "/wheels" mapping to work correctly
-				if (local.fullPath CONTAINS "/wheels") {
-					local.startPos = findNoCase("/wheels", local.fullPath);
-
-					// Prefer /vendor/wheels if available
-					local.vendorWheelsPos = findNoCase("/vendor/wheels/", local.fullPath);
-					if (local.vendorWheelsPos > 0) {
-						local.startPos = local.vendorWheelsPos + len("/vendor");
-					}
 
-					if (local.startPos > 0) {
-						local.fullPath = ExpandPath(mid(local.fullPath, local.startPos, len(local.fullPath) - local.startPos + 1));
+			// https://github.com/wheels-dev/wheels/issues/3077 — when the caller supplies an
+			// absolute `directory` (the documented "must be a full path … outside of the web
+			// root" contract) use it verbatim: build `fullPath` from the directory + file and
+			// skip both the `/wheels` mapping rewrite and the `ExpandPath()` fallback below.
+			// Those rewrites assume a relative, mapping-based path and otherwise (1) web-root-
+			// prefix the absolute path on Adobe CF — where `ExpandPath()` resolves against the
+			// web root rather than returning an absolute path unchanged as Lucee does — and
+			// (2) substring-hijack any directory containing "/wheels" (e.g. /var/www/wheels).
+			// The `..`-traversal guard above still applies to both arguments.
+			local.normalizedDir = Replace(arguments.directory, "\", "/", "all");
+			local.isAbsoluteDirectory = Len(arguments.directory) && (
+				Left(local.normalizedDir, 1) == "/" || REFind("^[A-Za-z]:", local.normalizedDir)
+			);
+
+			if (local.isAbsoluteDirectory) {
+				if (Len(local.normalizedDir) > 1 && Right(local.normalizedDir, 1) == "/") {
+					local.normalizedDir = Left(local.normalizedDir, Len(local.normalizedDir) - 1);
+				}
+				local.directory = local.normalizedDir;
+				local.file = arguments.file;
+				local.fullPath = local.directory & "/" & local.file;
+			} else {
+				if (Left(local.folder, Len(local.root)) == local.root) {
+					local.folder = RemoveChars(local.folder, 1, Len(local.root));
+				}
+				local.fullPath = Replace(local.folder, "\", "/", "all");
+				local.fullPath = ListAppend(local.fullPath, arguments.file, "/");
+				// https://github.com/wheels-dev/wheels/issues/873 Don't expand path if already contains root
+				if (local.fullPath DOES NOT CONTAIN Replace(local.root, "\", "/", "all")) {
+					//added this section for the "/wheels" mapping to work correctly
+					if (local.fullPath CONTAINS "/wheels") {
+						local.startPos = findNoCase("/wheels", local.fullPath);
+
+						// Prefer /vendor/wheels if available
+						local.vendorWheelsPos = findNoCase("/vendor/wheels/", local.fullPath);
+						if (local.vendorWheelsPos > 0) {
+							local.startPos = local.vendorWheelsPos + len("/vendor");
+						}
+
+						if (local.startPos > 0) {
+							local.fullPath = ExpandPath(mid(local.fullPath, local.startPos, len(local.fullPath) - local.startPos + 1));
+							local.fullPath = Replace(local.fullPath, "\", "/", "all");
+							local.file = ListLast(local.fullPath, "/");
+							local.directory = Reverse(ListRest(Reverse(local.fullPath), "/"));
+						}
+					} else{
+						local.fullPath = ExpandPath(local.fullPath);
 						local.fullPath = Replace(local.fullPath, "\", "/", "all");
 						local.file = ListLast(local.fullPath, "/");
 						local.directory = Reverse(ListRest(Reverse(local.fullPath), "/"));
 					}
-				} else{
-					local.fullPath = ExpandPath(local.fullPath);
-					local.fullPath = Replace(local.fullPath, "\", "/", "all");
-					local.file = ListLast(local.fullPath, "/");
-					local.directory = Reverse(ListRest(Reverse(local.fullPath), "/"));
 				}
 			}
 

vendor/wheels/tests/specs/controller/miscellaneousSpec.cfc

@@ -208,6 +208,59 @@ component extends="wheels.WheelsTest" {
 				expect(r.name).toBe("weirdname.png")
 			})
 
+			it("serves a file from an absolute directory outside the web root", () => {
+				// https://github.com/wheels-dev/wheels/issues/3077 — an absolute `directory`
+				// outside the web root must be used verbatim, not re-resolved via ExpandPath
+				// (which web-root-prefixes the path on Adobe CF).
+				local.outsideDir = GetTempDirectory() & "dlprobe3077_outside"
+				if (!DirectoryExists(local.outsideDir)) {
+					DirectoryCreate(local.outsideDir, true)
+				}
+				local.target = local.outsideDir & "/secret.txt"
+				FileWrite(local.target, "secret payload")
+				try {
+					args.file = "secret.txt"
+					args.directory = local.outsideDir
+					r = _controller.sendFile(argumentCollection = args)
+
+					expect(Replace(r.file, "\", "/", "all")).toInclude("dlprobe3077_outside/secret.txt")
+					expect(r.name).toBe("secret.txt")
+				} finally {
+					if (FileExists(local.target)) {
+						FileDelete(local.target)
+					}
+					if (DirectoryExists(local.outsideDir)) {
+						DirectoryDelete(local.outsideDir, true)
+					}
+				}
+			})
+
+			it("does not rewrite an absolute directory containing the '/wheels' substring", () => {
+				// https://github.com/wheels-dev/wheels/issues/3077 — the `/wheels` mapping
+				// fallback substring-matched ANY absolute path containing "/wheels"
+				// (e.g. /var/www/wheels/uploads), silently rewriting it.
+				local.wheelsDir = GetTempDirectory() & "wheels3077-dl"
+				if (!DirectoryExists(local.wheelsDir)) {
+					DirectoryCreate(local.wheelsDir, true)
+				}
+				local.target = local.wheelsDir & "/secret.txt"
+				FileWrite(local.target, "secret payload")
+				try {
+					args.file = "secret.txt"
+					args.directory = local.wheelsDir
+					r = _controller.sendFile(argumentCollection = args)
+
+					expect(Replace(r.file, "\", "/", "all")).toInclude("wheels3077-dl/secret.txt")
+				} finally {
+					if (FileExists(local.target)) {
+						FileDelete(local.target)
+					}
+					if (DirectoryExists(local.wheelsDir)) {
+						DirectoryDelete(local.wheelsDir, true)
+					}
+				}
+			})
+
 			it("is specifying a directory", () => {
 				// Skip this test temporarily to debug in CI
 				skip("Temporarily skipping to debug path issues in CI");