Bug #87733

unwanted side-effect in fileDenyPattern

Added by Stefan Frank 28 days ago. Updated 11 days ago.

Status:
Resolved
Priority:
Must have
Assignee:
-
Category:
Security
Target version:
-
Start date:
2019-02-18
Due date:
% Done:

100%

TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
medium
Is Regression:
No
Sprint Focus:

Description

The newly introduced fileDenyPattern

\.(php[3-7]?|phpsh|phtml|pht|phar|shtml|cgi|pl)(\..*)?$|^\.htaccess$

matches e.g. setup.pl.typoscript what appears to me as a suitable filename in a TypoScript cascade for a polish website. I have only limited knowledge in regex, but the following expression served its purpose in my tests.

\.(php[3-7]?|phpsh|phtml|pht|phar|shtml|cgi|pl)(\.*)?$|^\.htaccess$

Kind regards
Stefan


Related issues

Related to TYPO3 Core - Bug #24221: t3lib_TSparser::checkIncludeLines() does not check files to be included Closed 2010-11-28

Associated revisions

Revision 399a62a6 (diff)
Added by Oliver Hader 11 days ago

[BUGFIX] Reduce strictness for .pl files in fileDenyPattern

Files like "file.pl.txt" cannot be uploaded anymore since ".pl" is
considered as executable Perl file. In multilingual scenarios "pl"
is used as reference for Polish content. Since required modules
mod_perl or mod_cgid are not enabled by default, and limited to
be executable only when invoked in location "/cgi-bin/", now only
files ending with ".pl" (e.g. "file.pl") are denied.

Resolves: #87733
Releases: master, 9.5, 8.7
Change-Id: Ib9a69fd3ec04f51653857d2f7309e30b78932653
Reviewed-on: https://review.typo3.org/c/59886
Tested-by: TYPO3com <>
Tested-by: Anja Leichsenring <>
Tested-by: Susanne Moog <>
Tested-by: Markus Klein <>
Reviewed-by: Wolfgang Klinger <>
Reviewed-by: Anja Leichsenring <>
Reviewed-by: Susanne Moog <>
Reviewed-by: Markus Klein <>

Revision 8a63c209 (diff)
Added by Oliver Hader 11 days ago

[BUGFIX] Reduce strictness for .pl files in fileDenyPattern

Files like "file.pl.txt" cannot be uploaded anymore since ".pl" is
considered as executable Perl file. In multilingual scenarios "pl"
is used as reference for Polish content. Since required modules
mod_perl or mod_cgid are not enabled by default, and limited to
be executable only when invoked in location "/cgi-bin/", now only
files ending with ".pl" (e.g. "file.pl") are denied.

Resolves: #87733
Releases: master, 9.5, 8.7
Change-Id: Ib9a69fd3ec04f51653857d2f7309e30b78932653
Reviewed-on: https://review.typo3.org/c/59913
Tested-by: TYPO3com <>
Tested-by: Markus Klein <>
Reviewed-by: Markus Klein <>

Revision 0ae09f7d (diff)
Added by Oliver Hader 11 days ago

[BUGFIX] Reduce strictness for .pl files in fileDenyPattern

Files like "file.pl.txt" cannot be uploaded anymore since ".pl" is
considered as executable Perl file. In multilingual scenarios "pl"
is used as reference for Polish content. Since required modules
mod_perl or mod_cgid are not enabled by default, and limited to
be executable only when invoked in location "/cgi-bin/", now only
files ending with ".pl" (e.g. "file.pl") are denied.

Resolves: #87733
Releases: master, 9.5, 8.7
Change-Id: Ib9a69fd3ec04f51653857d2f7309e30b78932653
Reviewed-on: https://review.typo3.org/c/59914
Tested-by: TYPO3com <>
Tested-by: Markus Klein <>
Reviewed-by: Markus Klein <>

History

#1 Updated by Markus Klein 28 days ago

Can you please describe what the "unwanted side-effect" is?
What is your usecase, what does not work?

The regex modification of yours does not deny files like .pl.bak anymore, but it would still deny .pl.....

#2 Updated by Markus Klein 28 days ago

  • Status changed from New to Needs Feedback

#3 Updated by Stefan Frank 28 days ago

Markus Klein wrote:

Can you please describe what the "unwanted side-effect" is?
What is your usecase, what does not work?

Thank you for your attention to my request.

It is very reasonable to prevent files with, for example, the file extensions .bak and .sic from being delivered by the server. The .htaccess supplied with TYPO3 prevents the delivery of .bak files, so the fileDenyPattern is a "duplicate" if you use the supplied .htaccess. But I do not want to discuss or doubt your efforts in hardening TYPO3. My use case: At application level, I have stumbled across the fact that TYPO3 rejected the file setup.pl.typoscript. This file contains configuration data for a Polish website.

#4 Updated by Markus Klein 27 days ago

Sorry, I ask again. Please describe your setup.
Where do you use this setup.pl.typoscript file? Do you include it somewhere?

Give us instructions how to reproduce the issue.
Thanks

#5 Updated by Stefan Frank 25 days ago

Markus Klein wrote:

Where do you use this setup.pl.typoscript file? Do you include it somewhere?

In a multi-client system, the file setup.pl.ts is read in the root template of the client. The command is

<INCLUDE_TYPOSCRIPT: source="FILE:MYPATH/templates/ts/setup.ts">
<INCLUDE_TYPOSCRIPT: source="FILE:MYPATH/templates/ts/setup.pl.ts">

The file setup.pl.ts overwrites values from setup.ts.

Do you need further data?

Kind regards

#6 Updated by Markus Klein 25 days ago

Thanks for the info. Will take a look and will discuss, how to solve this problem properly.

#7 Updated by Markus Klein 25 days ago

  • Category set to Security
  • Status changed from Needs Feedback to Accepted
  • Assignee set to Markus Klein
  • Priority changed from Should have to Must have
  • Target version set to Candidate for patchlevel
  • Complexity set to medium
  • Is Regression set to Yes

#9 Updated by Oliver Hader 23 days ago

The modified file deny pattern would allow files like evil.php.jpg which might be executable depending on which web server and configuration is used.

In the last few years setups e.g. using the following in Apache have been "popular":

AddHandler php-script .php

Apache's approach for multiple file extensions is described here: https://httpd.apache.org/docs/2.4/mod/mod_mime.html#multipleext

Concerning setup.pl.typoscript and having a setup like documented in https://httpd.apache.org/docs/2.4/howto/cgi.html it would have to be considered as RCE vulnerability (that's why it was added to the deny pattern), basically:

AddHandler cgi-script .cgi .pl

#10 Updated by Oliver Hader 23 days ago

Thus, the the general question is why TypoScript includes are using the fileDeny pattern (in terms of parsing TypoScript configuration). It has been introduced in a security release a couple of years ago:

These are the aspects that should be tackled, replaced and hardened further.

#11 Updated by Oliver Hader 23 days ago

  • Is Regression changed from Yes to No

#12 Updated by Oliver Hader 23 days ago

  • Status changed from Accepted to Needs Feedback
  • Priority changed from Must have to Should have

#13 Updated by Oliver Hader 23 days ago

  • Target version deleted (Candidate for patchlevel)

#14 Updated by Oliver Hader 23 days ago

  • Related to Bug #24221: t3lib_TSparser::checkIncludeLines() does not check files to be included added

#15 Updated by Markus Klein 23 days ago

  • Assignee deleted (Markus Klein)

#16 Updated by Wolfgang Klinger 13 days ago

  • Priority changed from Should have to Must have

I stumbled upon this too right now …

We have a customer with clients in Poland (PL) and obviously uses filenames like

K.PL.BAN.A.V7.ACH.RES.pdf

and this does not work with the current regular expression in the last TYPO3 (neither 8, nor 9) versions, because ResourceStorage::checkFileExtensionPermission uses GeneralUtility::verifyFilenameAgainstDenyPattern which uses this pattern.

That's a real blocker, we can't tell clients: huh, you know, you can't use certain filenames when using TYPO3 ;-)

I mean it's easily solvable by changing the regular expression to match the extensions only after the last dot,
but that is not so obvious that you immediately think about it when this problem occurs.

#17 Updated by Gerrit Code Review 12 days ago

  • Status changed from Needs Feedback to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59886

#18 Updated by Gerrit Code Review 12 days ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59886

#19 Updated by Gerrit Code Review 12 days ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59886

#20 Updated by Gerrit Code Review 12 days ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59886

#21 Updated by Gerrit Code Review 11 days ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59913

#22 Updated by Gerrit Code Review 11 days ago

Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/59914

#23 Updated by Oliver Hader 11 days ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

Also available in: Atom PDF