diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d1ebf30..dfc1d63 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -78,7 +78,7 @@ jobs: strategy: max-parallel: 2 matrix: - php-versions: [7.4, '8.0'] + php-versions: ['7.4', '8.0'] typo3-versions: [10, 11] exclude: - diff --git a/Classes/Command/CheckContentEscapingCommand.php b/Classes/Command/CheckContentEscapingCommand.php new file mode 100644 index 0000000..3abfeeb --- /dev/null +++ b/Classes/Command/CheckContentEscapingCommand.php @@ -0,0 +1,312 @@ +setDescription( + 'Checks for possible escaping issues with content parameter due to new children escaping behavior' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $componentNamespaces = $this->componentLoader->getNamespaces(); + $templateFiles = $this->discoverTemplateFiles(); + + $progress = new ProgressBar($output, count($componentNamespaces) + count($templateFiles)); + $progress->start(); + + // Determine which components use {content -> f:format.raw()} or similar + foreach ($componentNamespaces as $namespace => $path) { + foreach ($this->componentLoader->findComponentsInNamespace($namespace) as $className => $file) { + try { + $template = $this->parseTemplate($file); + } catch (\TYPO3Fluid\Fluid\Core\Parser\Exception $e) { + $this->addResult($file, $e->getMessage(), true); + continue; + } + + if ($this->detectRawContentVariable($template->getRootNode())) { + $this->affectedComponents[$className] = $file; + } + } + $progress->advance(); + } + + // Check all templates for usage of the content parameter in combination with variables + foreach ($templateFiles as $file) { + $file = (string)$file; + try { + $template = $this->parseTemplate($file); + } catch (\TYPO3Fluid\Fluid\Core\Parser\Exception $e) { + $this->addResult($file, $e->getMessage(), true); + continue; + } + + $results = $this->detectEscapedVariablesPassedAsContent($template->getRootNode()); + foreach ($results as $result) { + $this->addResult($file, sprintf( + 'Component "%s" expects raw html content, but was called with potentially escaped variables: %s', + $this->cleanupPathForOutput($result[0]), + implode(', ', array_map(function ($variableName) { + return '{' . $variableName . '}'; + }, $result[1])) + )); + } + $progress->advance(); + } + + $progress->finish(); + + // Sort results alphabetically + ksort($this->results); + + // Output results + $output->writeln(''); + foreach ($this->results as $file => $messages) { + $output->writeln(''); + $output->writeln(sprintf( + '%s:', + $this->cleanupPathForOutput($file) + )); + $output->writeln(''); + + foreach ($messages as $message) { + $output->writeln($message); + $output->writeln(''); + } + } + + return 0; + } + + public function detectRawContentVariable(NodeInterface $node, array $parents = []): bool + { + $node = $this->resolveEscapingNode($node); + + $lastParent = count($parents) - 1; + foreach ($node->getChildNodes() as $childNode) { + $childNode = $this->resolveEscapingNode($childNode); + + // Check all parent elements of content variable + if ($childNode instanceof ObjectAccessorNode && $childNode->getObjectPath() === 'content') { + for ($i = $lastParent; $i >= 0; $i--) { + // Skip all non-viewhelpers + if (!($parents[$i] instanceof ViewHelperNode)) { + continue; + } + + // Check for f:format.raw + if ($parents[$i]->getUninitializedViewHelper() instanceof RawViewHelper) { + return true; + } + } + } + + // Check if the slot ViewHelper is present + if ($childNode instanceof ViewHelperNode) { + $viewHelper = $childNode->getUninitializedViewHelper(); + if ($viewHelper instanceof SlotViewHelper) { + return true; + } + } + + // Search for more occurances of content variable + $result = $this->detectRawContentVariable($childNode, array_merge($parents, [$childNode])); + if ($result) { + return true; + } + } + + return false; + } + + public function detectEscapedVariablesPassedAsContent(NodeInterface $node): array + { + $node = $this->resolveEscapingNode($node); + + $results = []; + foreach ($node->getChildNodes() as $childNode) { + $childNode = $this->resolveEscapingNode($childNode); + + // Check if a component was used + if ($childNode instanceof ViewHelperNode) { + $viewHelper = $childNode->getUninitializedViewHelper(); + if ($viewHelper instanceof ComponentRenderer && + isset($this->affectedComponents[$viewHelper->getComponentNamespace()]) + ) { + // Check if variables were used inside of content parameter + $contentNode = $childNode->getArguments()['content'] ?? $childNode; + $variableNames = $this->checkForVariablesWithoutRaw($contentNode); + if (!empty($variableNames)) { + $results[] = [ + $this->affectedComponents[$viewHelper->getComponentNamespace()], + $variableNames + ]; + } + continue; + } + } + + $results = array_merge( + $results, + $this->detectEscapedVariablesPassedAsContent($childNode) + ); + } + + return $results; + } + + public function checkForVariablesWithoutRaw(NodeInterface $node, array $parents = []): array + { + $node = $this->resolveEscapingNode($node); + + $variableNames = []; + $lastParent = count($parents) - 1; + foreach ($node->getChildNodes() as $childNode) { + $childNode = $this->resolveEscapingNode($childNode); + + // Check all parent elements of variables + if ($childNode instanceof ObjectAccessorNode && + !in_array($childNode->getObjectPath(), static::IGNORED_VARIABLES) + ) { + for ($i = $lastParent; $i >= 0; $i--) { + // Skip all non-viewhelpers + if (!$parents[$i] instanceof ViewHelperNode) { + continue; + } + + // Check for f:format.raw etc. + $viewHelper = $parents[$i]->getUninitializedViewHelper(); + if (in_array(get_class($viewHelper), static::RAW_VIEWHELPERS)) { + continue 2; + } + } + + $variableNames[] = $childNode->getObjectPath(); + continue; + } + + // Search for more occurances of variables + $variableNames = array_merge( + $variableNames, + $this->checkForVariablesWithoutRaw($childNode, array_merge($parents, [$childNode])) + ); + } + + return $variableNames; + } + + protected function discoverTemplateFiles(): array + { + // All extensions in local extension directory + $activeExtensions = array_filter($this->packageManager->getActivePackages(), function ($package) { + return strpos($package->getPackagePath(), Environment::getExtensionsPath()) === 0; + }); + + // All template paths (Resources/Private/) + $possibleTemplatePaths = array_map(function ($package) { + return ExtensionManagementUtility::extPath($package->getPackageKey(), 'Resources/Private/'); + }, $activeExtensions); + $possibleTemplatePaths = array_filter($possibleTemplatePaths, 'file_exists'); + + // Find all html files + $finder = new Finder(); + $finder + ->in($possibleTemplatePaths) + ->files()->name('*.html'); + return iterator_to_array($finder); + } + + protected function addResult(string $file, string $message, bool $isError = false): void + { + $this->results[$file] ??= []; + $format = ($isError) ? '%s' : '%s'; + $this->results[$file][] = sprintf($format, $message); + } + + protected function cleanupPathForOutput(string $path): string + { + return trim(str_replace(Environment::getProjectPath(), '', $path), '/'); + } + + protected function resolveEscapingNode(NodeInterface $node): NodeInterface + { + return ($node instanceof EscapingNode) ? $this->resolveEscapingNode($node->getNode()) : $node; + } + + protected function parseTemplate(string $file): ParsingState + { + $this->templates[$file] ??= $this->getTemplateParser()->parse( + file_get_contents($file), + $file + ); + return $this->templates[$file]; + } + + protected function getTemplateParser(): TemplateParser + { + return (new StandaloneView())->getRenderingContext()->getTemplateParser(); + } + + public function injectPackageManager(PackageManager $packageManager): void + { + $this->packageManager = $packageManager; + } + + public function injectComponentLoader(ComponentLoader $componentLoader): void + { + $this->componentLoader = $componentLoader; + } +} diff --git a/Classes/Domain/Model/Slot.php b/Classes/Domain/Model/Slot.php new file mode 100644 index 0000000..9001020 --- /dev/null +++ b/Classes/Domain/Model/Slot.php @@ -0,0 +1,34 @@ +html = $html; + } + + public static function fromString(string $html): Slot + { + return new Slot($html); + } + + public function count(): int + { + return strlen($this->html); + } + + public function __toString(): string + { + return $this->html; + } +} diff --git a/Classes/Fluid/ViewHelper/ComponentRenderer.php b/Classes/Fluid/ViewHelper/ComponentRenderer.php index 611f387..ddcb842 100644 --- a/Classes/Fluid/ViewHelper/ComponentRenderer.php +++ b/Classes/Fluid/ViewHelper/ComponentRenderer.php @@ -3,7 +3,9 @@ namespace SMS\FluidComponents\Fluid\ViewHelper; use Psr\Container\ContainerInterface; +use SMS\FluidComponents\Domain\Model\Slot; use SMS\FluidComponents\Interfaces\ComponentAware; +use SMS\FluidComponents\Interfaces\EscapedParameter; use SMS\FluidComponents\Interfaces\RenderingContextAware; use SMS\FluidComponents\Utility\ComponentArgumentConverter; use SMS\FluidComponents\Utility\ComponentLoader; @@ -79,7 +81,7 @@ class ComponentRenderer extends AbstractViewHelper * * @var boolean */ - protected $escapeChildren = false; + protected $escapeChildren = true; /** * @var ComponentLoader @@ -190,6 +192,11 @@ public function render() ]); $variableContainer->add('settings', $this->componentSettings); + // Provide component content to renderer + if (!isset($this->arguments['content'])) { + $this->arguments['content'] = (string)$this->renderChildren(); + } + // Provide supplied arguments from component call to renderer foreach ($this->arguments as $name => $argument) { $argumentType = $this->argumentDefinitions[$name]->getType(); @@ -209,11 +216,6 @@ public function render() $variableContainer->add($name, $argument); } - // Provide component content to renderer - if (!isset($this->arguments['content'])) { - $variableContainer->add('content', $this->renderChildren()); - } - // Initialize component rendering template if (!isset($this->parsedTemplate)) { $componentFile = $this->componentLoader->findComponent($this->componentNamespace); @@ -300,8 +302,11 @@ public function initializeArguments() ); $this->registerArgument( 'content', - 'string', - 'Main content of the component; falls back to ViewHelper tag content' + Slot::class, + 'Main content of the component; falls back to ViewHelper tag content', + false, + null, + true ); $this->initializeComponentParams(); @@ -456,7 +461,8 @@ protected function initializeComponentParams() $optional = $param['optional'] ?? false; $description = $param['description'] ?? ''; - $this->registerArgument($param['name'], $param['type'], $description, !$optional, $param['default']); + $escape = is_subclass_of($param['type'], EscapedParameter::class) ? true : null; + $this->registerArgument($param['name'], $param['type'], $description, !$optional, $param['default'], $escape); } } } diff --git a/Classes/Interfaces/EscapedParameter.php b/Classes/Interfaces/EscapedParameter.php new file mode 100644 index 0000000..76c4820 --- /dev/null +++ b/Classes/Interfaces/EscapedParameter.php @@ -0,0 +1,22 @@ + + * + * + * + * + */ +interface EscapedParameter +{ +} diff --git a/Classes/ViewHelpers/SlotViewHelper.php b/Classes/ViewHelpers/SlotViewHelper.php new file mode 100644 index 0000000..27aee8d --- /dev/null +++ b/Classes/ViewHelpers/SlotViewHelper.php @@ -0,0 +1,53 @@ +registerArgument('name', 'string', 'Name of the slot that should be rendered', false, 'content'); + $this->registerArgument( + 'default', + 'string', + 'Default content that should be rendered if slot is not defined (falls back to tag content)', + false, + null, + true + ); + } + + /* + * @param array $arguments + * @param \Closure $renderChildrenClosure + * @param RenderingContextInterface $renderingContext + * @return string + */ + public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext) + { + $slotContent = $renderingContext->getVariableProvider()->get($arguments['name']); + + if (isset($slotContent) && !$slotContent instanceof Slot) { + throw new InvalidArgumentException( + sprintf('Slot "%s" cannot be rendered because it isn\'t a valid slot object.', $arguments['name']), + 1670247849 + ); + } + + if ((string)$slotContent === '') { + return $arguments['default'] ?: $renderChildrenClosure(); + } + + return $slotContent; + } +} diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml index 6c4b1b0..d329e28 100644 --- a/Configuration/Services.yaml +++ b/Configuration/Services.yaml @@ -15,3 +15,11 @@ services: description: 'Generates the XSD files for autocompletion in the IDE.' schedulable: true hidden: false + + SMS\FluidComponents\Command\CheckContentEscapingCommand: + tags: + - name: 'console.command' + command: 'fluidcomponents:checkContentEscaping' + description: 'Checks for possible escaping issues with content parameter due to changed children escaping behavior' + schedulable: false + hidden: false diff --git a/Documentation/DataStructures.md b/Documentation/DataStructures.md index 7e8016b..a0f1280 100644 --- a/Documentation/DataStructures.md +++ b/Documentation/DataStructures.md @@ -101,7 +101,7 @@ of the URI by using `parse_url`. You get access to the following link properties * `query`: e. g. `myparam=1` * `fragment`: e. g. `myfragment` -## Files Images +## Files and Images Components should be able to accept a file or image as a parameter, no matter where it come from. TYPO3 already has data structures for files that are stored in the File Abstraction Layer. However, there are cases where you want to use @@ -382,6 +382,37 @@ default: date: 1594110573 ``` +## Slots + +Similar to frontend frameworks [such as Vue.js](https://vuejs.org/guide/components/slots.html), Slots are a data structure that allows you to pass +arbitrary HTML markup to a component. The default `{content}` parameter is implemented as a slot, but you can add your own slots. Slots behave +differently compared to other parameters in the way escaping works. If a parameter defined as a Slot gets passed a Fluid variable, the content +of that variable will be escaped, which makes sure that malicious user input doesn't lead to XSS injections. + +To output a slot, you should **always** use the `` ViewHelper which takes care of a safe HTML output. **format.raw() should no longer be used in Fluid Components since v3.5.0!** + +Fluid Component `Molecule/TeaserCard/TeaserCard.html`: + +```xml + + + + + + +<> +``` + +```xml + + + + + + + +``` + ## Type Aliases The included data structures can also be defined with their alias. These are `Image`, `Link`, `Typolink`, `Labels`, `Navigation` and `NavigationItem`. diff --git a/Documentation/Forms.md b/Documentation/Forms.md index ac98c08..90b8f58 100644 --- a/Documentation/Forms.md +++ b/Documentation/Forms.md @@ -41,7 +41,7 @@ Components/Molecule/FieldLabel/FieldLabel.html: