Path Manipulation (security vulnerability)

Question

A Fortify security review informed us of some path manipulation vulnerabilities. Most have been obvious and easy fixes, but I don't understand how to fix the following one.

string[] wsdlFiles = System.IO.Directory.GetFiles(wsdlPath, "*.wsdl");

"wsdlPath" is input from a textbox. Is this something that just can't be fixed? I can validate the path exists, etc. but how is that helping the vulnerability?

Quadrille

How is that code run? If it is a Windows application that runs with the credentials of the user entering the wsdlPath I see nothing wrong. If it runs in a Windows Service or as part of a web site it is a problem. – Rainfall

Did the Fortify review provide the injected string? – Cathar

@AndersAbel - web app. an authenticated user enters the path, and if it's a valid path then it's accepted. – Quadrille

@Cathar - they didn't provide the string, they assume any string could be potentially bad. – Quadrille

Answer 1

If the data is always obtained from a text box whose contents are determined by the user, and the code runs using the permissions of that user, then the only threat is that of the user attacking themselves. That is not an interesting threat.

The vulnerability which the tool is attempting to alert you to is that if low-trust hostile code can determine the contents of that string then the hostile code can mount an attempt to discover facts about the user's machine, like "is such and such a program that I happen to know has a security vulnerability installed and unpatched?" or "is there a user named 'admin' on this machine?" and so on.

Lucrative

The only user that can access this page is a system admin. There are just a few people with admin level access. I suppose this is enough "damage control" to consider this vulnerability "low". I just don't see any way to make this stronger than it is. Thanks for your input. – Quadrille

I marked this as the answer because the authenticated user inputting this information isn't going to obtain any useful information to them. An end-user, yes absolutely... but that isn't the case here. – Quadrille

@Quadrille - you make it stronger by ensuring the input is filtered to only allow valid path and filenames. Yes, admin access only does mitigate things, but what if an attacker gains admin access, this could allow them to gain access to the server itself, not just your app. – Spurling

@MystereMan: If the attacker gains admin access by some means then the attacker does not need to attack via the app. The horse is already inside the walls of Troy and the attackers are opening the gates for their army of Myrmidions to invade. Defense in depth is a good idea, but hostile admins will simply bypass your defenses; that's what being an admin means: being admin is precisely possessing the ability to bypass the normal rules that keep the system secure. – Lucrative

That said, there is some value in differentiating between site admins and server admins. However, it is easy to inadvertently create a security hole which turns a site admin into a de facto server admin. – Britisher

@EricLippert - Not necessarily. Having admin access to the app does not give them admin access to the server. – Spurling

@MystereMan: Sure. Let's suppose that there is some vulnerability whereby a site admin may become a server admin. The ability of the site admin to determine filenames on their own machine does not help them become a server admin. But let's suppose it does, somehow. Preventing that from happening in this program does not prevent the site admin from doing so by other means; they are the admin. – Lucrative

@EricLippert - determining filenames could help to guess usernames, which the attacker could then try dictionary password attacks against via remote desktop or some other remote connection. Or what if there there is a vulnerability in the GetFiles API that allows arbitrary code execution? – Spurling

@MystereMan: Yes, absolutely. But if the attacker is already a site admin then they do not need to use this application to enumerate the user names. They can just enumerate the user names directly. And if the attacker is already an admin then they do not need to use an exploit to execute arbitrary code: they are the admin; they already can execute arbitrary code directly. – Lucrative

@EricLippert - i get the feeling we're talking past each other here. How can a web site admin enumerate server users directly? At most they could enumerate web site users (those stored in the database, not the web servers users) – Spurling

Hello Everyone(@EricLippert @MystereMan), Please help me in the below question as i'm finding difficulties to fix the coverity issue - [#66455578 – Hawfinch

Answer 2

You should never feed anything directly into OS API's unfiltered. You should sanitize the input, make sure it doesn't contain paths (ie "../../../somefile" And make sure it truncates long names, and contains only valid filename characters (for instance, there have been various bugs relating to international characters).

Spurling

The input is being handled as it should. This was obviously red flagged because it originates from user input. But that needs to remain intact. To me, it looks like there is nothing we can do here short of eliminating user input, which isn't an option. Maybe somebody can see something here that I can't. – Quadrille

@Quadrille - Your response is baffling. What do you mean by "the input is being handled as it should"? Your comments indicate that it's not. You must sanitize the input to make sure you aren't feeding illegal data into an OS api, or paths to files that should not be allowed. If you think it's perfectly acceptable for a user to input, say the system password file, then hey.. forget i said anything. – Spurling

Answer 3

With that code, any user that is authenticated and authorized to use that function, is able to access the file system on the server. The access will be done using the credentials of the service account that runs the web application.

Depending on how the returned data is used, a malicious user might be able to get more information or make the server behave in a way that was not intended.

You should limit the set of allowed paths to only consist of one or a few carefully selected directories. Use the functions in the Path class to combine strings into paths - they take care of things like a user entering c:\allowedpath\..\windows\system32 for you.

Rainfall

Answer 4

This kind of scenarios needs encoding and decoding to make sure that data is not manipulated anywhere. Because while decryption if data is changed you will get the wrong results.

You can create your encoding and decoding. I did it using RijndaelManaged and PasswordDeriveBytes classes provided by System.Security.Cryptography;

Valerievalerio