File location wih String format

Question

In Java, are these the same:

new File (a.getPath() + filename);

new File (String.format(a.getPath() + filename));

I'm getting some warning in findbugs, and the 2nd option seems to fix it.

Answer 1

Both are bad options IMO. If the idea is to combine a directory name with a filename, use the File constructor which is designed specifically for that:

new File(a.getPath(), filename)

(I can't see why using String.format would fix the warning, but as we don't know what the warning is, it's hard to guess. It would definitely make it more fragile though, as the string would be interpreted as a format pattern, which presumably it isn't.)

Answer 2

String.format() doesn't do anything unless your string is a format string and you add extra arguments, so in this case it looks like you're just hiding the warning. Most likely FindBugs knows that new File(a + b); is probably a mistake, but can't tell if new File(someFunction()); is a mistake.

According to the documentation for PT_RELATIVE_PATH_TRAVERSAL:

The software uses an HTTP request parameter to construct a pathname that should be within a restricted directory, but it does not properly neutralize sequences such as ".." that can resolve to a location that is outside of that directory. See http://cwe.mitre.org/data/definitions/23.html for more information.

FindBugs looks only for the most blatant, obvious cases of relative path traversal. If FindBugs found any, you almost certainly have more vulnerabilities that FindBugs doesn't report. If you are concerned about relative path traversal, you should seriously consider using a commercial static analysis or pen-testing tool.

So, the String.format() trick works because FindBugs can only catch obvious cases of this, but the bigger problem appears to be that this causes a security vulnerability:

This allows attackers to traverse the file system to access files or directories that are outside of the restricted directory.

It's not clear exactly what's causing FindBugs to show this warning, since you didn't post your code, but it sounds like it could be pretty serious. You probably want to actually fix this instead of ignoring the warning.

Answer 3

1. Those are the same.

2. Using String.format to pass-through a static value is weird. Don't do it.

3. Not everything Findbugs reports is a problem. Findbugs can report "false positives."

As suggested above, use new File(a.getPath(), filename) to add a path segment to another path. That may also fix your Findbugs issue.