The three previous articles shared a pattern: the user's data reached an interpreter and became structure. Database command, page markup, system command.
Path Traversal breaks that pattern. There is no interpreter, no syntax being hijacked. The code does exactly one thing: it opens the file it was asked for. The problem is that the one choosing which file is the user, and they can choose one you did not have in mind.
Path Traversal: the path that climbs
File systems have an old convention: .. means the directory above. It exists so you can write relative paths, and it works anywhere a path is accepted.
var path = Path.Combine(_storageRoot, fileName);
return PhysicalFile(path, "application/octet-stream");That PhysicalFile is a method from ASP.NET Core, not from the C# language. It serves a file from an absolute path on disk.
With fileName holding july-sales.pdf, the code delivers the report. With fileName holding ../../appsettings.json, it leaves the storage folder, climbs two levels and delivers the application configuration file, connection strings included.
And it does not stop there. Four levels up and into a system folder leaves the application entirely. The limit is not the folder you had in mind, it is whatever the process account can read.
Path.Combine is not a defense, and in one case it works against you
There is a belief that Path.Combine is the safe way to join paths, as opposed to concatenating with +. It is the correct way, in the sense of handling slashes properly across operating systems. But it validates nothing.
Path.Combine("storage", "../../appsettings.json") returns exactly storage/../../appsettings.json, without complaining. It joins the pieces, and resolving the .. happens later, in the file system.
And it has a behaviour that tends to surprise people: if the second piece is an absolute path, the first is discarded.
Path.Combine("storage", "/etc/passwd")That does not return storage/etc/passwd. It returns /etc/passwd. The folder you thought you were pinning disappears from the result, and the user did not even need to use .. to escape. On Windows, the equivalent with an absolute drive path has the same effect.
So in practice, that way of building the path offers no protection at all.
The fix: resolve the path and check where it landed
Since the problem is that you do not know where the path points, the fix is to find out before opening the file. Two steps.
First, resolve the path to its final form, with Path.GetFullPath. It processes the .., normalizes the slashes and returns the real absolute path. After it, storage/../../appsettings.json stops hiding where it goes.
Second, check that this final path is still inside the allowed folder. If it is not, you refuse.
var root = Path.GetFullPath("storage") + Path.DirectorySeparatorChar;
var path = Path.GetFullPath(Path.Combine(root, fileName));
if (!path.StartsWith(root, StringComparison.Ordinal))
return Forbid();
return PhysicalFile(path, "application/octet-stream");The detail of the separator at the end
Notice that + Path.DirectorySeparatorChar on the first line. It looks decorative and it is not.
The most direct way to write that comparison is like this:
var root = Path.GetFullPath("storage");
if (!path.StartsWith(root, StringComparison.Ordinal))The catch is that StartsWith compares text, and does not understand folder hierarchy. If your root is /app/storage, the path /app/storage-backup/passwords.txt starts with /app/storage from the text point of view. The check passes, and a file from a neighbouring folder is handed over.
Adding the separator at the end of the root fixes it, because then the comparison requires the path to really be inside it: /app/storage-backup/... does not start with /app/storage/.
The shorter road: do not let the user pick the name
The fix above is necessary when the user really does need to say which file they want. But in a good share of cases they do not.
If what they have is a report identifier, and the file name on disk is your business, the vulnerability does not exist:
var report = await repository.GetByIdAsync(reportId);
if (report is null)
return NotFound();
return PhysicalFile(Path.Combine(_storageRoot, report.StoredFileName), "application/pdf");Here the name comes from your database, not from the request. The user picks which record, and you pick which file. There is no incoming path to validate.
The same applies on the upload side: instead of storing the file under the name the user sent, store it under a name you generate and keep the original name as metadata.
var storedName = Guid.NewGuid() + Path.GetExtension(file.FileName);
var path = Path.Combine(_storageRoot, storedName);
await using var output = System.IO.File.Create(path);
await file.CopyToAsync(output);A quick note on that System.IO.File: inside an ASP.NET Core controller, writing just File does not compile, because File there resolves to a method of the controller itself. Qualifying it solves it, and it is the kind of detail that costs somebody ten minutes.
And when the user really only needs to point at a simple name, with no folder at all, there is a direct tool: Path.GetFileName discards any directory part of what it receives. Path.GetFileName("../../appsettings.json") returns appsettings.json, and the .. is thrown away before getting anywhere near the disk.
Why it goes unnoticed
The code has nothing strange about it. There is no suspicious concatenation, no special character, no interpreter involved. Path.Combine(root, fileName) is the most reasonable line in the world, and one many people would write in a code review without raising an eyebrow.
And like the two previous vulnerabilities, the normal test passes. You test with the name of a file that exists in the folder, it downloads, done. The alternative path only shows up if somebody thinks of typing .., and there is no reason to think of that while you are validating the feature.
What this becomes in training
Every pattern here goes in as a pair, same as the previous articles. And this is the category where the classifier came out most unbalanced: it is quite confident when it flags Path Traversal, and at the same time it misses four out of every ten cases it should catch.
That makes sense, and the reason is this whole article: the difference between the vulnerable and the safe version here is not one extra function in the middle of the line, as it was with the encoding in Cross-Site Scripting. It is a three line check that happens before, and sometimes it is just the fact that the file name comes from the database instead of the request. That is context, not shape, and shape is what the model sees.
Hold on to that observation, because it is the central argument of the article about the second opinion.
The next article closes the fundamentals with Insecure Deserialization, the least known of the five and the one that gives whoever exploits it the most direct result: code execution.