[$1000] uploading EPS extension file throws an error

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open any chat
2. Click on ➕ icon
3. Click on Add attachment
4. Select .EPS extension file and send it

Expected Result:

The app should send the file or throw "This filetype is not allowed" pop-up.

Actual Result:

Gives an error saying "Invalid image supplied"

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.53-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

2023-07-31.00-06-33.mp4

Expensify/Expensify Issue URL:
Issue reported by: @jo-ui
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690751127702569

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01441ec377d38bb1cc
• Upwork Job ID: 1693709065932484608
• Last Price Increase: 2023-08-21

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[ishpaul777]

Proposal

Problem

uploading EPS extension file throws an error

Root cause

We are not validating dropped file extension while we validate isValidFile.

https://github.com/Expensify/App/blob/main/src/components/AttachmentModal.js#L186

Changes

Ideally all files with any type possible should not be supported. (IMO, should never support executable files like ".exe", ".msi"). Slack and google chat also blocks certain type of file uploads ref here and here

Validating before uploading to backend is generally a good idea. We should add a condition to validate file extension.

rough implementation -

 const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', ''));
    if (_.contains(CONST.API_ATTACHMENT_VALIDATIONS.UNALLOWED_EXTENSIONS, fileExtension.toLowerCase())) {
        setIsAttachmentInvalid(true);
            setAttachmentInvalidReasonTitle('attachmentPicker.fileTypeNotAllowed'Title);
            setAttachmentInvalidReason('attachmentPicker.fileTypeNotAllowedBody');
            return false;
    }

Alternatives

What we can do is only allow files with a set of extension and revert above condition.

 const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', ''));
    if (!_.contains(CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_EXTENSIONS, fileExtension.toLowerCase())) {
        setIsAttachmentInvalid(true);
            setAttachmentInvalidReasonTitle('attachmentPicker.fileTypeNotAllowed'Title);
            setAttachmentInvalidReason('attachmentPicker.fileTypeNotAllowedBody');
            return false;
    }

[DylanDylann]

I think It is BE bug. The error is threw from BE side

[jo-ui]

@strepanier03 any updates on this issue?

[jo-ui]

@strepanier03 It has been a week since this issue was posted, but there has been no response

[strepanier03]

I was on sabbatical and am just getting back from it now. I'm working on this today and will have an update soon.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01441ec377d38bb1cc

[melvin-bot]

Current assignee @strepanier03 is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @burczu (External)

[strepanier03]

I was able to recreate this behavior using a random .eps file on my computer as well as the one shared in the Slack bug report thread.

[strepanier03]

@burczu - If you think this is an internal issue please let me know and I can switch to an Internal label.

[b4s36t4]

I can upload the files without any issue on staging.

[burczu]

@strepanier03 The Invalid image supplied message that is added after the Unexpeceted error... message comes from the BE side, so I think it means that BE has some issue processing files with this extension. That's why I believe we should we should make it internal and at least check what's happening with these files on the BE side.

[ishpaul777]

@burczu I agree that error comes from BE.
But IMO allowing uploads for all file type to backend could potentially expose the application to security vulnerabilities. What do you think?

[burczu]

@ishpaul777 from what I see the app allows to upload any type of file - if it's not an image or pdf then you can just download the file (there is no preview in the report) and I guess it is done this way by design

[ishpaul777]

Platforms like Slack, Gmail and Google Chat have implemented restrictions on certain types of file uploads (references: Slack policy, Gmail and Google Chat policy). Considering this, it might be prudent for us to implement similar restrictions on the types of files users can upload. I recommend that we discuss and consider implementing a file type validation mechanism that only allows safe and necessary file types for upload.
Allowing all file types, including potentially harmful ones like executable files (e.g., ".exe") exposes vulnerabilities for users as well as backend, However, if this is intentionally part of the design, I recommend that we document the rationale behind it, along with the assessed risks.

[strepanier03]

@burczu - reading the response from @ishpaul777 here, do you want to raise this for discussion at all or should I switch the labels now?

[burczu]

@strepanier03 - I think for this issue we should just switch the labels

@ishpaul777 - I can hear you and it's reasonable what you wrote, but I think it sounds more like a feature request so I believe you could raise a separate issue to discuss it - what do you think?

[ishpaul777]

Yes we can switch label for this one. Can you chime in slack thread https://expensify.slack.com/archives/C01GTK53T8Q/p1692691281183839 sharing your thoughts?

[strepanier03]

Still waiting for internal ownership, I'll bump again internally this week.

[dangrous]

Hey @strepanier03 I had a second to look into this - I'm not seeing the error, on staging or production. Are you still able to reproduce? A sample .eps file I used uploads and downloads fine.

Not sure what's changed since the issue was opened but we might be all set now.

[jo-ui]

@dangrous Try it with other .eps files.
EPS files.zip

[strepanier03]

@dangrous - I'll give it another shot.

[strepanier03]

@dangrous - Just tested again with the three .eps files that @jo-ui shared here. All three repro'd the reported behavior.

[dangrous]

Ah yep, tried some others. Making a PR to fix

[dangrous]

Not overdue, fix is merged but not yet deployed

[dangrous]

This should be fixed! @strepanier03 do you mind testing when you have a moment? I might be able to get to it later this week

[strepanier03]

Yup, I can test. Doing it now and will update shortly.

[strepanier03]

I'm able to upload the files successfully and download them, but I don't have a program on my mac to open them. When I try it opens in ATOM, which isn't a vector file program.

The behavior looks updated and correct if I had a program to open an EPS file though.

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[dangrous]

Okay I think I'm gonna close this out!

[jo-ui]

@strepanier03 It is working perfectly on both staging and production. Can you send me an offer for the reporting bonus

[dangrous]

My bad! Reopening to handle payment.

[strepanier03]

Thanks @dangrous - I'll work on the reporting payment and close out when it's wrapped.

[strepanier03]

@jo-ui - I can't figure out who you are in Upwork. Can you apply for the job here or link your Upwork profile and I'll hire you?

[jo-ui]

@strepanier03 I've applied for the job on upwork

[melvin-bot]

@dangrous, @burczu, @strepanier03 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[strepanier03]

Thanks @jo-ui - I have hired you and will keep an eye out for your acceptance so I can pay this and get this closed out today.

[jo-ui]

Thank you @strepanier03 I've accepted the offer on upwork

[strepanier03]

I've paid out the contract, ended it, and left feedback.

Thank you @jo-ui for being part of this community 🙌