Java applications, including web applications, that accept file uploads must ensure that an attacker cannot upload or transfer malicious files. If a restricted file containing code is executed by the target system, it can compromise application-layer defenses. For example, an application that permits HTML files to be uploaded could allow malicious code to be executed—an attacker can submit a valid HTML file with a cross-site scripting (XSS) payload that will execute in the absence of an output-escaping routine. For this reason, many applications restrict the type of files that can be uploaded.

It may also be possible to upload files with dangerous extensions such as .exe and .sh that could cause arbitrary code execution on server-side applications. An application that restricts only the Content-Type field in the HTTP header could be vulnerable to such an attack.

To support file upload, a typical Java Server Pages (JSP) page consists of code such as the following:

<s:form action="doUpload" method="POST" enctype="multipart/form-data">
  <s:file name="uploadFile" label="Choose File" size="40" />
  <s:submit value="Upload" name="submit" />
</s:form>

Many Java enterprise frameworks provide configuration settings intended to be used as a defense against arbitrary file upload. Unfortunately, most of them fail to provide adequate protection. Mitigation of this vulnerability involves checking file size, content type, and file contents, among other metadata attributes.

Noncompliant Code Example

This noncompliant code example shows XML code from the upload action of a Struts 2 application. The interceptor code is responsible for allowing file uploads.

<action name="doUpload" class="com.example.UploadAction">
  <interceptor-ref name="upload">
    <param name="maximumSize"> 10240 </param>
    <param name="allowedTypes"> text/plain,image/JPEG,text/html </param>
  </interceptor-ref>        
</action>

The code for file upload appears in the UploadAction class:

public class UploadAction extends ActionSupport {
  private File uploadedFile;
  // setter and getter for uploadedFile
  
  public String execute() {
    try {
      // File path and file name are hardcoded for illustration
      File fileToCreate = new File("filepath", "filename");
      // Copy temporary file content to this file
      FileUtils.copyFile(uploadedFile, fileToCreate);
      return "SUCCESS";
    } catch (Throwable e) {
      addActionError(e.getMessage());
      return "ERROR";
    }
  }
} 

The value of the parameter type maximumSize ensures that a particular Action cannot receive a very large file. The allowedTypes parameter defines the type of files that are accepted. However, this approach fails to ensure that the uploaded file conforms to the security requirements because interceptor checks can be trivially bypassed. If an attacker were to use a proxy tool to change the content type in the raw HTTP request in transit, the framework would fail to prevent the file's upload. Consequently, an attacker could upload a malicious file that has a .exe extension, for example.

Compliant Solution 

The file upload must succeed only when the content type matches the actual content of the file. For example, a file with an image header must contain only an image and must not contain executable code. This compliant solution uses the Apache Tika library [Apache 2013]  to detect and extract metadata and structured text content from documents using existing parser libraries. The checkMetaData() method must be called before invoking code in execute() that is responsible for uploading the file.

public class UploadAction extends ActionSupport {
  private File uploadedFile;
  // setter and getter for uploadedFile
  
  public String execute() {
    try {
      // File path and file name are hardcoded for illustration
      File fileToCreate = new File("filepath", "filename");

      boolean textPlain = checkMetaData(uploadedFile, "text/plain");
      boolean img = checkMetaData(uploadedFile, "image/JPEG");
      boolean textHtml = checkMetaData(uploadedFile, "text/html");

	  if (!textPlain && !img && !textHtml) {
	    return "ERROR";
      }		

	  // Copy temporary file content to this file
      FileUtils.copyFile(uploadedFile, fileToCreate);
      return "SUCCESS";
    } catch (Throwable e) {
      addActionError(e.getMessage());
      return "ERROR";
    }
  }

  public static boolean checkMetaData(
    File f, String getContentType) {
    try (InputStream is = new FileInputStream(f)) {
      ContentHandler contenthandler = new BodyContentHandler();
      Metadata metadata = new Metadata();
      metadata.set(Metadata.RESOURCE_NAME_KEY, f.getName());
      Parser parser = new AutoDetectParser();
      try {
        parser.parse(is, contenthandler, metadata, new ParseContext());
      } catch (SAXException | TikaException e) {
        // Handle error
        return false;
      }
      
      if (metadata.get(Metadata.CONTENT_TYPE).equalsIgnoreCase(getContentType)) {
        return true;
      } else {
        return false;
      }
    } catch (IOException e) {
      // Handle error
      return false;
    }
  }
}

The AutoDetectParser selects the best available parser on the basis of the content type of the file to be parsed.

Applicability

An arbitrary file upload vulnerability could result in privilege escalation and the execution of arbitrary code.

Automated Detection

ToolVersionCheckerDescription
The Checker Framework

2.1.3

Tainting CheckerTrust and security errors (see Chapter 8)

Bibliography

 


12 Comments

  1. I have also added the 2 required jars as attachments.

    You will also need tika-app-1.3.jar for the CS to compile. Could not get that to upload. Perhaps because confluence implements this guideline correctly and that is an executable JAR. 8-)

  2. This is a good start at a new rule. Comments:

    • It should probably mention that it is about web apps...I trust we're not concerned with desktop apps that are given untrusted files to open, right?
    • Don't understand this paragraph under the NCCE:

    However, this approach does not ensure that the uploaded file conforms to the security requirements as interceptor checks can be trivially bypassed. If an attacker uses a proxy tool to change the content type in the raw HTTP request in transit, the framework would not prevent the file's upload.

    Need more details...does this mean an attacker can upload a forbidden file type such as .exe?

    Or does the NCCE example permit an attacker to embed neferious data in a benign file? (eg a .jpeg that contains a malicious executable)? Whatever the answer is, it can go in the intro, with a summary in the Applicability section.

    • I also suspect this rule belongs in the IDS category, as it seems to be about proper sanitization of files uploaded to a web app. The Applicability section.
  3. Text now uses subjunctive tense for future hypotheticals; also moved a few "only"s for precision of expression.

  4. This statement seems misordered or perhaps just wrong:

    If a restricted file containing code is executed by the target system, it can result in misuse of privileges.

     

     

  5. In the NCE, I feel like the name of the method being invoked is "fileUpload" but I cannot find a method by this name in the Java class.

  6. I feel like the compliant solution is missing a big chunk.  It says that "The checkMetaData() method must be called before invoking execute()." but where is execute called?  Perhaps we should show this code as well?

  7. I'm vaguely worried about the statement:

    "It may also be possible to upload files with dangerous extensions such as .exe and .sh which could cause arbitrary code execution on server-side applications."

    I guess on Windows, a file becomes executable based on it's extension.  On UNIX, any file can be executed, if it has execute permissions.  I'm wondering how important the extension is?  I think there are all sorts of sneaky attacks where it looks like a file has an OK extension but still causes code to execute.  The vul team guys might know more about these.  If the file is uploaded with another extension, could the attacker possibly "rename" the file?  I'm worried this rule appears a bit naive.

  8. The rule seems to be biased towards Windows platforms.  We should probably try to even it out a little more.

  9. Received, via email:

    Submitted by: Giovanni Cerrato
    Date: 12/20/2016
    Reason for Contacting Us: Ask a Question, Provide Feedback
    Message:
    Hello,

    I am using some snippets of code provided by your web site.
    The compliant solution of "IDS56-J. Prevent arbitrary file upload" at this URL:
    ...
    In particular this code:
    if (!textPlain || !img || !textHtml) [{ return "ERROR";}
    should be:
    if (!textPlain && !img && !textHtml)
    This if statment verifies that users try to upload a non permitted file type. An error should be returned if the detected content type is different to all allowed types, so the right version should be use the Java AND operator (&&) instead of the OR operator (||).
    Could you check? if you need any further information don\'t hesitate to contact me. Thanks

    Giovanni

    1. My response:

      Giovanni:

      I inspected the code in rule IDS56-J. You are quite correct, the condition should have been joined by && rather than ||. As written, the code will always return ERROR, which is incorrect.
      I have applied your suggestion to the guideline on the wiki...thanks for the report!

  10. Is there a reason not to use the DefaultDetector only and instead using the AutoDetectParser? The following method seems to me more straight forward and does exactly the same as the above method:


    public boolean checkMetaData(File f, String getContentType) {

        try (TikaInputStream is = TikaInputStream.get(new FileInputStream(f))) {

            Metadata metadata = new Metadata();

            metadata.set(Metadata.RESOURCE_NAME_KEY, f.getName());

            DefaultDetector detector = new DefaultDetector();

            MediaType mediaType = detector.detect(is, metadata);

            if (mediaType.equals(MediaType.parse(getContentType))) {

                return true;

            } else {

                return false;

            }

        } catch (IOException e) {

            // Handle error

            return false;

        }

    }