-3

Why is this code considered dangerous or vulnerable?

String sFileName = request.getParameter("fName");
if (sFileName.toLowerCase().endsWith(".pdf"))
// open file
else
// don’t open the file

closed as unclear what you're asking by Steffen Ullrich, Xander, Neil Smithline, kasperd, sebastian nielsen May 20 '16 at 22:12

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    Please take a look at the help center before asking a question. – Insane May 19 '16 at 5:33
  • 1
    @Insane: Is your comment aimed at the bad formatting of the question or the topic in general? Because I found the question and especially the answer by fbo3264 to be very interesting. Just curious. – hamena314 May 19 '16 at 7:28
  • 2
    @hamena314 Both. Considered by who? More details in general? And of course, the bad formatting. Also because the snippet looks like a direct rip from this SO question. – Insane May 19 '16 at 7:37
  • SafetyNetter, are you asking about this comment: "It's generally dangerous to run PDFs downloaded from the internet from the local filesystem" There are lots of reasons why it could be dangerous, but it's difficult to narrow it down for you without context. – schroeder May 19 '16 at 15:29
  • Who considers this dangerous? What is the context that it is run in? What does "open file" mean? Is it opened on the server? Sent as part of the requestt? Other? – Neil Smithline May 20 '16 at 1:55
1

Apart from path traversal attacks this code could be vulnerable to Null-Byte injection. For more information read this.

Update:

As Maarten Bodewes pointed out, this has been fixed since Java 1.7.0_40. I remember this being a vulnerability in 1.6 though. Anyway when you take a look at the source code for the File class you find that the isInvalid() method has been added which does the null-byte check:

final boolean isInvalid() {
    if (status == null) {
        status = (this.path.indexOf('\u0000') < 0) ? PathStatus.CHECKED
                                                   : PathStatus.INVALID;
    }
    return status == PathStatus.INVALID;
}

See also this post on stackoverflow.

  • 1
    This is a very light answer, where the bulk of the answer is in a link. Can you include the relevant parts of the link in your answer? – schroeder May 19 '16 at 15:34
  • 1
    A quick search found that null-byte injection was fixed in 1.7 b40. – Maarten Bodewes May 19 '16 at 17:42
0

Right now I see 2 problems with this code, Depending on the context.

  • The fact that the filename ends with .pdf does not mean it is a PDF file. Which could lead to all kinds of trouble.
  • Malicious code can be hidden in macros of PDF files.

But like I said, it all depends on the context.

Not the answer you're looking for? Browse other questions tagged or ask your own question.