Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Authoring): Check real mime type when author uploads a file #187

Merged
merged 2 commits into from
Nov 13, 2022

Conversation

geoffreykwan
Copy link
Member

Changes

Check a file's real MIME type when an author uploads a file to the File Manager.

Test

  1. Download the Burp Suite Community Edition (no need to enter email, just click "Go straight to downloads")
    https://portswigger.net/burp/communitydownload

  2. Install Burp Suite Community Edition

  3. Launch Burp Suite Community Edition

  4. Leave "Temporary project" selected

    • Click "Next"
  5. Use Burp defaults

    • Click "Start Burp"
  6. Configure the proxy

    • Click Proxy tab
    • Click Options tab
    • Under Proxy Listeners, highlight the 127.0.0.1:8080 line
    • Click "Edit"
    • Change port from 8080 to 8081
  7. Launch the browser

    • Click Intercept tab
    • Click "Open browser"
  8. Upload a file that should not be allowed

    • In the Burp browser go to localhost:81
    • Log in as a teacher
    • Open a unit in the Authoring Tool
    • Go to the File Manager view
    • Go back to the Burp application and click "Intercept is off" to turn intercept on
    • Go back to the Burp browser and drag and drop a php file into the File Manager
    • Go back to the Burp application and it should have intercepted the upload request
    • Change
      • Content-Type: application/octet-stream
      • to
      • Content-Type: image/jpeg
    • Click "Forward" button at the top left
    • The file should be rejected. Previously the file would be accepted.

Closes #186

@geoffreykwan geoffreykwan self-assigned this Nov 4, 2022
@geoffreykwan geoffreykwan marked this pull request as ready for review November 7, 2022 20:33
Copy link
Member

@hirokiterashima hirokiterashima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality works as described, but I had some questions about the code. See inline comments.

return allowedTypes.contains(getRealMimeType(file));
}

public String getRealMimeType(MultipartFile file) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public String getRealMimeType(MultipartFile file) {
private String getRealMimeType(MultipartFile file) {

Comment on lines 130 to 137
try {
Metadata metadata = new Metadata();
TikaInputStream stream = TikaInputStream.get(file.getInputStream());
org.apache.tika.mime.MediaType mediaType = detector.detect(stream, metadata);
return mediaType.toString();
} catch (IOException e) {
return MimeTypes.OCTET_STREAM;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add OCTET_STREAM to allowedTypes in application.properties, could this allow someone to potentially upload a file that we do not support?

To handle this case, should this method throw the IOException instead of returning a default OCTET_STREAM, and catch the exception in isUserAllowedToUpload() method and return false there?

@geoffreykwan geoffreykwan merged commit db0d003 into develop Nov 13, 2022
@geoffreykwan geoffreykwan deleted the issue-186-file-upload-check-real-mime-type branch November 13, 2022 22:40
@geoffreykwan
Copy link
Member Author

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Check real mime type when author uploads a file
3 participants