CODEWITHSUNDEEP
javajava-security
Sanyam Madaan
Sanyam Madaan

Reputation: 139

Securely read a file in java

I am scanning my code with IBM AppScan, and in one method I am passing file uploaded by user of UploadedFile type and reading it to a byte array using below code. But the scan gives a "Vulnerability Type - Validation.Required" error. I am verifying the file extension and null checking the file before I run this piece of code.

Validation check: 

if (file != null && !file.getFileName().isEmpty()) {
     // Checking file extension here. Like jpg,png etc.
}

It falls in category CWE 20 if it helps. https://cwe.mitre.org/data/definitions/20.html

private int fileUpload(UploadedFile file) { // import org.primefaces.model.UploadedFile;
        try {
            String fileName = file.getFileName();
            int fileSize = (int) file.getSize();
            String fileType = new Validator().fetchFileExtension(fileName);
            byte[] filebytea = new byte[fileSize];
            try {
                FileInputStream fileStream = (FileInputStream) file.getInputstream();
                fileStream.read(filebytea); // Error: Vulnerability Type Validation.Required
            } catch (Exception e) {
                //System.out.println("error in file stream" + e.getMessage());
            }

Upvotes: 2

Views: 3401

Answers (2)

Robert
Robert

Reputation: 42575

Your call to read is the problem. It can but does not have to read the data fully. Hence you have to check the return value of read and call it multiple times. If you don't want to implement this yourself you can use DataInputStream.readFully() instead or fileStream.readAllBytes() (the latter requires Java 9+).

The following code should not cause any problems and works on Java 7+:

    try (DataInputStream in = new DataInputStream(file.getInputstream())) {
        in.readFully(filebytea); 
    } catch (Exception e) {
        //System.out.println("error in file stream" + e.getMessage());
    }

Upvotes: 2

AndiCover
AndiCover

Reputation: 1734

Don't use FileInputStream or FileOutputStream. They are considered as harmful. In some cases rhey can lead to GC problems which you don't want in production.

See this explanation. SonarQube typically shows this issue as well.

Instead you should use:

Files.newInputStream(..)
Files.newOutputStream(..)

Upvotes: 0

Related Questions