[!!!][TASK] Remove import of PHP files of admins from impexp 21/61221/3
authorBenni Mack <benni@typo3.org>
Wed, 3 Jul 2019 15:47:44 +0000 (17:47 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Wed, 3 Jul 2019 18:19:17 +0000 (20:19 +0200)
The functionality, which was only allowed for admins, does not
consider FAL restrictions. In order to be consistent with FAL, this
is removed.

Resolves: #88681
Releases: master
Change-Id: I7a42539b5391af7d730deabffbe638a2ceed1a05
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61221
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/core/Documentation/Changelog/master/Breaking-88681-ImportOfPHPFilesInImportExportFilesRemoved.rst [new file with mode: 0644]
typo3/sysext/impexp/Classes/Controller/ImportExportController.php
typo3/sysext/impexp/Classes/Import.php
typo3/sysext/impexp/Classes/ImportExport.php
typo3/sysext/impexp/Resources/Private/Language/locallang_csh.xlf
typo3/sysext/impexp/Resources/Private/Partials/Import/Import.html

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-88681-ImportOfPHPFilesInImportExportFilesRemoved.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-88681-ImportOfPHPFilesInImportExportFilesRemoved.rst
new file mode 100644 (file)
index 0000000..f3a7db5
--- /dev/null
@@ -0,0 +1,37 @@
+.. include:: ../../Includes.txt
+
+=====================================================================
+Breaking: #88681 - Import of PHP files in Import/Export files removed
+=====================================================================
+
+See :issue:`88681`
+
+Description
+===========
+
+Importing XML data via `EXT:impexp` previously allowed to import PHP files for Administrators
+in TYPO3 Backend. This by-pass functionality is removed, and the configured File Deny Pattern
+now applies for all imports in order to streamline import functionality with other file
+operations within TYPO3 Core.
+
+
+Impact
+======
+
+Importing XML files with embedded PHP files via EXT:impexp will trigger an import error and disallow
+the import of the file.
+
+
+Affected Installations
+======================
+
+Any TYPO3 installations using the data importer that use import files with included PHP files.
+
+
+Migration
+=========
+
+Ensure to include PHP files into a custom local extension, as importing PHP code is highly
+discouraged - even for administrators.
+
+.. index:: PHP-API, NotScanned, ext:impexp
\ No newline at end of file
index ae18085..39e4804 100644 (file)
@@ -818,7 +818,6 @@ class ImportExportController
             $import->global_ignore_pid = $inData['global_ignore_pid'];
             $import->force_all_UIDS = $inData['force_all_UIDS'];
             $import->showDiff = !$inData['notShowDiff'];
-            $import->allowPHPScripts = $inData['allowPHPScripts'];
             $import->softrefInputValues = $inData['softrefInputValues'];
 
             // OUTPUT creation:
index dd3b1d8..1477cec 100644 (file)
@@ -1541,7 +1541,7 @@ class Import extends ImportExport
             }
         }
         $fI = GeneralUtility::split_fileref($fileName);
-        if (!$fileProcObj->checkIfAllowed($fI['fileext'], $fI['path'], $fI['file']) && (!$this->allowPHPScripts || !$this->getBackendUser()->isAdmin())) {
+        if (!$fileProcObj->checkIfAllowed($fI['fileext'], $fI['path'], $fI['file'])) {
             $this->error('ERROR: Filename "' . $fileName . '" failed against extension check or deny-pattern!');
             return false;
         }
index b89be66..65521d6 100644 (file)
@@ -139,13 +139,6 @@ abstract class ImportExport
     public $showDiff = false;
 
     /**
-     * If set, and if the user is admin, allow the writing of PHP scripts to fileadmin/ area.
-     *
-     * @var bool
-     */
-    public $allowPHPScripts = false;
-
-    /**
      * Array of values to substitute in editable softreferences.
      *
      * @var array
@@ -771,11 +764,11 @@ abstract class ImportExport
                 $fileProcObj = $this->getFileProcObj();
                 if ($fileProcObj->actionPerms['addFile']) {
                     $testFI = GeneralUtility::split_fileref(Environment::getPublicPath() . '/' . $fI['relFileName']);
-                    if (!$this->allowPHPScripts && !$fileProcObj->checkIfAllowed($testFI['fileext'], $testFI['path'], $testFI['file'])) {
+                    if (!$fileProcObj->checkIfAllowed($testFI['fileext'], $testFI['path'], $testFI['file'])) {
                         $pInfo['msg'] .= 'File extension was not allowed!';
                     }
                 } else {
-                    $pInfo['msg'] = 'You user profile does not allow you to create files on the server!';
+                    $pInfo['msg'] = 'Your user profile does not allow you to create files on the server!';
                 }
             }
             $pInfo['showDiffContent'] = PathUtility::stripPathSitePrefix($this->fileIDMap[$ID]);
index 63b1c50..ba6b061 100644 (file)
@@ -324,20 +324,13 @@ EXT:impexp/Resources/Public/Images/cshimages/update.png</source>
                        <trans-unit id="options.details" xml:space="preserve">
                                <source>&lt;b&gt;Do not show differences in records&lt;/b&gt;
 When a structure has been imported you will see a difference view of all records which tells you whether the written content matched the import data or if not, what changed. In many cases content &lt;i&gt;should&lt;/i&gt; change (eg. relations and file references) so it is not an error if you see red and green values. Basically the difference view is a feature you can use for visual validation of the import success. It brings piece-of-mind to those who is enlightened to understand what it tells... :-)
-Green strings represent the actual written data while red represents the original value from the import file and black represents data that is the same.
-
-&lt;b&gt;Allow to write banned file extensions (eg. PHP scripts), if any&lt;/b&gt;
-Also an option for admins-only; Allows PHP-files (for example from soft references in TypoScript templates) to be written to the system. This is normally not allowed behavior for security reasons.</source>
+Green strings represent the actual written data while red represents the original value from the import file and black represents data that is the same.</source>
                        </trans-unit>
                        <trans-unit id="_options.image" xml:space="preserve">
                                <source>EXT:impexp/Resources/Public/Images/cshimages/impexp_misc3.png,
 EXT:impexp/Resources/Public/Images/cshimages/phpext.png</source>
                                <note from="developer">This string contains an internal text, which must not be changed. Just copy the original text into the translation field. For more information have a look at the Tutorial.</note>
                        </trans-unit>
-                       <trans-unit id="options.image_descr" xml:space="preserve">
-                               <source>Options for import.
-Here you are told that this PHP file cannot be allowed - because you didn't select the option "Allow to write banned file extensions (eg. PHP scripts), if any"</source>
-                       </trans-unit>
                        <trans-unit id="action.alttitle">
                                <source>Import Action</source>
                        </trans-unit>
index 76df2de..cae73cd 100644 (file)
     <p class="help-block">(<f:translate key="importdata_greenValuesAreFrom" />)</p>
 </div>
 <f:if condition="{isAdmin}">
-    <div class="form-group">
-        <label for="checkAllowPHPScripts">
-            <f:form.checkbox name="tx_impexp[allowPHPScripts]" id="checkAllowPHPScripts" value="1"
-                                            checked="{inData.allowPHPScripts} == 1" />
-            <f:translate key="importdata_allowToWriteBanned" />
-        </label>
-    </div>
     <f:if condition="!{inData.do_update}">
         <div class="form-group">
             <label for="checkForce_all_UIDS">