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

Default fileExtension regex does not match plain files. #8698

Closed
4 tasks done
dblythy opened this issue Jul 23, 2023 · 9 comments · Fixed by #8699
Closed
4 tasks done

Default fileExtension regex does not match plain files. #8698

dblythy opened this issue Jul 23, 2023 · 9 comments · Fixed by #8699
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@dblythy
Copy link
Member

dblythy commented Jul 23, 2023

New Issue Checklist

Issue Description

The current default fileExtension regex ("^[^hH][^tT][^mM][^lL]?$") does not match plain.

Steps to reproduce

  1. Do not set fileExtension
  2. Try to create file

Actual Outcome

Error

Expected Outcome

Should succeed

Environment

Server

  • Parse Server version: FILL_THIS_OUT
  • Operating system: FILL_THIS_OUT
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): FILL_THIS_OUT

Database

  • System (MongoDB or Postgres): FILL_THIS_OUT
  • Database version: FILL_THIS_OUT
  • Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): FILL_THIS_OUT

Client

  • SDK (iOS, Android, JavaScript, PHP, Unity, etc): FILL_THIS_OUT
  • SDK version: FILL_THIS_OUT

Logs

@parse-github-assistant
Copy link

Thanks for opening this issue!

  • ❌ Please fill out all fields with a placeholder FILL_THIS_OUT, otherwise your issue will be closed. If a field does not apply to the issue, fill in n/a.

@dblythy
Copy link
Member Author

dblythy commented Jul 23, 2023

Related: parse-community/Parse-SDK-JS#1979

@dblythy
Copy link
Member Author

dblythy commented Jul 23, 2023

I think the default regex should instead be '^(?!html$|htm$).*'

@mtrezza
Copy link
Member

mtrezza commented Jul 23, 2023

From the web server point of view, the path component of a URI is case sensitive, so we cannot add the i flag to the regex. On the other hand modern web browsers handle file extensions case-insensitive (file.HTML, file.html, file.HtMl, etc. are all rendered as HTML).

So to truly prevent HTML content files we'd need to block any case-style without using the i flag, that's why we had the regex so far.

If we don't keep the current case insensitive regex style, then the regex could be simply ^(?!html?$). This requires only 4 steps for the regex engine, regardless of the extension string length. But that would allow uploading malicious.HTmL and browsers would likely render the HTML content.

If we want to keep the case insensitive style then we could use ^(?!(h|H)(t|T)(m|M)(l|L)?$), which should

  • not match html, htm with any casing combination (e.g. html, HTML HtMl)
  • match empty string '', i.e. no file extension
  • match any other extension

Btw, the current file extension regex looks wrong anyway as it won't allow extensions shorter than 3 or longer than 4 chars.

@dblythy
Copy link
Member Author

dblythy commented Jul 24, 2023

Is it possible that a file could be uploaded with the extension hTmL?

@mtrezza
Copy link
Member

mtrezza commented Jul 24, 2023

If the file extension regex is ["^(?!html?$)"] without the i flag, then yes, and a browser would render it. But we can't use the i flag, because we would limit developers who differentiate casing in the URI, technically index.HTML and index.html could be 2 different files. So we'd need ^(?!(h|H)(t|T)(m|M)(l|L)?$).

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.0-alpha.26

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Mar 10, 2024
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Mar 19, 2024
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
3 participants