For Unrestricted FileUpload, indicate the file was too large to upload

[preetkaran20]

Is your feature request related to a problem? Please describe. Currently, VulnerableApp's UnrestrictedFileUpload vulnerability is not having any indicator for telling users that the uploaded file is too large which causes trouble where users start thinking that the setup has some issues, etc. Take a file of say 5-6 MB and try to upload it, you will see failures.

Describe the solution you'd like we should notify the user that the uploaded file is too large hence we cannot upload it.

Describe alternatives you've considered Nothing.

Additional context Files to look for:

1. https://github.com/SasanLabs/VulnerableApp/tree/master/src/main/java/org/sasanlabs/service/vulnerability/fileupload (Backend)
2. https://github.com/SasanLabs/VulnerableApp/tree/master/src/main/resources/static/templates/UnrestrictedFileUpload/LEVEL_1 (UserInterface)

[agnarrarendelle]

Hi, I am new to Open Source contribution and would liken to work on this one, but there are a few things that I want to know. What is the maximum file size to upload? In what files should I put the new code in and what to do after users upload files that are too large? Throw an exception or simply return from that function

[preetkaran20]

Hi @agnarrarendelle ,

Great !!!, I think if you upload a large file then Spring boot should automatically return 413 Request Entity Too Large status in response so, in the https://github.com/SasanLabs/VulnerableApp/tree/master/src/main/resources/static/templates/UnrestrictedFileUpload/LEVEL_1 file, you can parse the response and if it is 413, you can simply indicate in UI that uploaded file is too large, please choose smaller file (or something like that).

I think the maximum file size to upload is configured in Spring boot properties but we are having a default limit.

[agnarrarendelle]

Ok, I plan to do something like this in FileUpload.js:

var file = form.files[0];

//my code

if (file.size > "maxSpringBootSize"){

 //return from that function and create a warning window to indicate that the file is too large

   }

Where can I find the default limit size in the project? Sorry if I am asking too many questions because I am still fairly new to open source project contribution

[preetkaran20]

No no, actually, It is really good that you asked these questions as it helps in thinking differently.

We can go with the default limit of 10MB, and spring-boot has https://docs.spring.io/spring-boot/docs/current/api/org/springframework/boot/autoconfigure/web/servlet/MultipartProperties.html#:~:text=The%20default%20is%2010MB.,will%20be%20written%20to%20disk limit.

So One important thing is if we have this check as you have added then it can be bypassed from OWASP ZAP/direct API call etc, so we should have a check on response status as well which is very simple like response.status == 413, then show a message in UI. You can add the check you mentioned along with my suggestion. Also, I would not suggest pop up's as they impact the user experience.

[preetkaran20]

why I am suggesting to also add a post check where the server does the check, is because we want to introduce a vulnerability where the backend doesn't have any restriction on the size of the uploaded file as it can cause DOS which is a great addition to set of Vulnerabilities. (I added https://github.com/SasanLabs/VulnerableApp/issues/351 after your comment)

[agnarrarendelle]

Hi, I just cloned the repo on my local environment, but when I tried to run it it says: "Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0. Use '--warning-mode all' to show the individual deprecation warnings.", and when I tried to open http://localhost:9090/VulnerableApp, it failed to open. This is strange because it also says "BUILD SUCCESSFUL" My IDE is Intellij

Edit: I ran gradle build again and it is currently installing some files, but it still didn't work

[agnarrarendelle]

I tried to build again and this is the error message:

> Configure project :
The AbstractArchiveTask.baseName property has been deprecated. This is scheduled to be removed in Gradle 8.0. Please use the archiveBaseName property instead. See https://docs.gradle.org/7.4/dsl/org.gradle.api.tasks.bundling.AbstractArchiveTask.html#org.gradle.api.tasks.bundling.AbstractArchiveTask:baseName for more details.
        at build_8yzye8iue4pwcdn13zn2yyj9o$_run_closure1.doCall(/home/matt/Open_Source_Project_Contribution/VulnerableApp/build.gradle:24)
        (Run with --stacktrace to get the full stack trace of this deprecation warning.)
The AbstractArchiveTask.version property has been deprecated. This is scheduled to be removed in Gradle 8.0. Please use the archiveVersion property instead. See https://docs.gradle.org/7.4/dsl/org.gradle.api.tasks.bundling.AbstractArchiveTask.html#org.gradle.api.tasks.bundling.AbstractArchiveTask:version for more details.
        at build_8yzye8iue4pwcdn13zn2yyj9o$_run_closure1.doCall(/home/matt/Open_Source_Project_Contribution/VulnerableApp/build.gradle:25)
        (Run with --stacktrace to get the full stack trace of this deprecation warning.)

FAILURE: Build failed with an exception.

* What went wrong:
Task 'VulnerableApp' not found in root project 'VulnerableApp'.

[preetkaran20]

Hi @agnarrarendelle,

To build the application execute:

1. ./gradlew build

To Run the project there are 2 ways:

1. execute: ./gradlew jibDockerBuild and then as mentioned in https://github.com/SasanLabs/VulnerableApp#building-the-project
2. execute ./gradlew bootRun for running as standalone. If you want to run from IDE then go to https://github.com/SasanLabs/VulnerableApp/blob/master/src/main/java/org/sasanlabs/Application.java#L14 and run from Intellij.

[agnarrarendelle]

Thanks for the help, I have successfully made it run. And, I also would like to know if my way of checking response status is correct

  document.getElementById("upload").addEventListener("click", function () {
  var httpResponse = new XMLHttpRequest()
  if(httpResponse.status == 413){
    //change UI to say that file too big
  return;
  }

Also, as I am currently not running it in Docker, the response status is always 0, so I might try Docker tomorrow to see if I can get normal response status Thank you for all the help, and sorry for so many questions irrelevant to the issues that I was going to handle

[preetkaran20]

Response status is 0 because you are not calling the endpoint from your code. It should work for both Docker and directly from IDE. However, overall code structure looks good.

No Issues, thank you.

[agnarrarendelle]

Hi, I have made the succeeded in implementing an error message to prevent user to upload files that are bigger than 10mb but I could not have got correct response status(it is still always 0). I have tried to use both getXMLHttpRequest() function and create a XMLHttpRequest() variable myself but they did not work, so I would like to ask for more guidance, but for now I have made the function to reject files larger than 10mb that are uploaded via submit button

var file = form.files[0];
if(file.size > 10485760){
      let fileSizeMB = (file.size/1024)/1024
      fileSizeMB = fileSizeMB.toFixed(3)
      document.getElementById("uploaded_file_info").innerHTML ="File Size " + fileSizeMB + "MB Is Too Big! Please choose a smaller file"
      document.getElementById("uploaded_file_info").style.color = "red";
      return
     }else{
      document.getElementById("uploaded_file_info").innerHTML = ""
      document.getElementById("uploaded_file_info").style.color = "black";
     }

And this is my failed attempt to use XMLHttpRequest(). I saw it was used in similar way in another function, so I copied the code but it did not work

let xmlHttpRequest = new XMLHttpRequest();
xmlHttpRequest.onreadystatechange = function () {
    genericResponseHandler(xmlHttpRequest, uploadImage, true);
};
xmlHttpRequest.open("GET", url, true);
xmlHttpRequest.setRequestHeader(
    "Content-Type",
   "application/json"
  );
xmlHttpRequest.send();

Could I make a pull request for now to merge these changes?

[preetkaran20]

actually you need to create another callback function similar to genericResponseHandler in fileupload.js which adds another check for 413 response code. Look at: https://github.com/SasanLabs/VulnerableApp/blob/d377e22acda179bcc775329c3e9814d60e3207d1/src/main/resources/static/vulnerableApp.js#L221