Do not interfere with PDF files

[Hlavtox]

Questions	Answers
Branch?	8.1.x
Description?	There was a security "thing" that forced PDF file downloads instead of opening it in browser. This could be good for securing public - uploaded PDFs, but not the ones from merchant. I removed adding this rule to the global root .htaccess and added it only to uploads/.htaccess, where all these uploads go.
Type?	bug fix
Category?	CO
BC breaks?	no
Deprecations?	no
How to test?	See below
Fixed ticket?	Fixes #31602
Related PRs
Sponsor company

Research - user untrusted file uploads

• Message attachments (orderd detail and contact page) - PS_UPLOAD_DIR
• Product customizations - PS_UPLOAD_DIR

How to test

• _Should be probably tested by a developer, @Robin-Fischer-PS what do you think?
• Clone this.
• Regenerate htaccess file by saving any page meta configuration or delete this bit of rules from htaccess.
• Copy any pdf file to root directory of prestashop and try to open it in browser, it will respect settings of your browser and probably display it, instead of forcing download of the file.
• Copy any pdf file to /uploads directory and try to open it in browser. Should be forced downloaded.

Disclaimer

It behaves in a weird way sometimes. I am using XAMPP on Windows and either Apache or Chrome is caching it.

• I have a aaa.pdf in my prestashop root. When I access it in browser, it downloads.
• Then I delete the rule from .htaccess.
• When I access aaa.pdf again, it's still downloading instead of displaying.
• When I rename aaa.pdf to bbb.pdf and access it, it opens in browser.
• BUT, when I access aaa.pdf I still get it and it gets downloaded, even though it doesnt exist anymore. :-) 🤷‍♂️

[MatShir]

@Hlavtox milestone 8.1 but targeting develop ? 🤔

The base branch was changed.

[Hlavtox]

@matthieu-rolland @nicosomb Reapproval pls, I owe you a beer

@MatShir fixed :D

[Robin-Fischer-PS]

Hi ! It can be tested by a dev indeed. Thanks @Hlavtox !

[jolelievre]

Warning!

This was introduced ON PURPOSE in a security fix GHSA-rc8c-v7rq-q392

So until we're sure this doesn't reopen a fixed security breach I prefer to block this. Besides I don't see why it's such a big issue not to be able to open the PDF in the browser, I can understand its "a bit" annoying but in face of security it's completely negligeable

[Hlavtox]

@jolelievre OK, then this should be only put to upload directory .htaccess, so we block user uploaded PDFs, but not the ones in other folders. 👍

[0x346e3730]

I don't understand how a server config is related to security, what if someone decides to use nginx or caddy to host PrestaShop ? Does it mean the "security fix" does not work ?

[matthieu-rolland]

So if understand well:

1. The security measure is to set all PDF files as attached files so that they are never opened in the browser, thus avoiding any risk of XSS vulnerability.

2. This is a problem for merchants who want to have their own PDF files opened in the browser directly, not as attachments.

About PDFs uploaded in the BO:

today a customer warned me on the fact that the pdf files that he had placed in a server folder, in the webroot of the site, he could not view them from the browser but made them download

If we don't trust a PDF uploaded by a merchant or an employee, why would we trust a PDF directly put into the server ?
If we consider that an uploaded PDF is not trustworthy because the BO could have been hacked, then the same could be said about the server.

• I think we should trust all PDFs not uploaded by a customer. It's the merchant's responsibility to not use corrupted PDF files.

About PDFs uploaded by a customer in the FO:

• In all cases, customer-uploaded PDFs shouldn't be opened in the browser, for security reasons.

Solution proposal:

• find all use cases where a PDF can be uploaded by a customer
• see where those PDFs are stored in those cases, and add the apache rule in the htaccess file at this place.
• the apache rule can be removed in Tools.php

[Hlavtox]

User untrusted file uploads

• Message attachments (orderd detail and contact page) - PS_UPLOAD_DIR
• Product customizations - PS_UPLOAD_DIR

Consulted on Slack

[davidglezz]

What if someone decides to use nginx or caddy to host PrestaShop ? Does it mean the "security fix" does not work ?

I was wondering the same thing. Does anyone know the answer?

[matthieu-rolland]

What if someone decides to use nginx or caddy to host PrestaShop ? Does it mean the "security fix" does not work ?

I was wondering the same thing. Does anyone know the answer?

Oh I missed this question:

Well by default PrestaShop provides htaccess configurations, for Apache. If one chooses not to use Apache but instead Nginx or Caddy, then he has to replicate those configurations himself.

Our documentation provides a configuration example for Nginx: https://devdocs.prestashop-project.org/8/basics/installation/nginx/

you can see in the example that the upload folder is dealt with in this file, when this PR is merged we should update this documentation

[Hlavtox]

@hibatallahAouadni There is nothing to consult here, you can test it

[florine2623]

I add the label Waiting for dev to follow this message #31637 (comment)

[tleon]

Hello I tested this as of today. It's all good for me. I indeed encountered the same behavior in chrome as described but changing the name of the file did the tick.

It's a go for me.

[Hlavtox]

Thank you Thomas!!! :-)

@kpodemski We can merge