diff --git a/app/Util/HtmlPurifier/ConfiguredHtmlPurifier.php b/app/Util/HtmlPurifier/ConfiguredHtmlPurifier.php index 221db6ffc..c4dd91778 100644 --- a/app/Util/HtmlPurifier/ConfiguredHtmlPurifier.php +++ b/app/Util/HtmlPurifier/ConfiguredHtmlPurifier.php @@ -3,6 +3,7 @@ namespace BookStack\Util\HtmlPurifier; use BookStack\App\AppVersion; +use BookStack\Util\HtmlPurifier\Filters\UriEnsureScheme; use BookStack\Util\HtmlPurifier\Filters\UriLimitFileProtocolToAnchors; use BookStack\Util\UrlFilter; use HTMLPurifier; @@ -84,14 +85,18 @@ class ConfiguredHtmlPurifier $config->set('Attr.EnableID', true); $config->set('Attr.ID.HTML5', true); $config->set('Output.FixInnerHTML', false); - $config->set('URI.SafeIframeRegexp', '%^(http://|https://|//)%'); $allowedSchemes = UrlFilter::getAllowedSchemes(); $allowedSchemesSetting = []; foreach ($allowedSchemes as $scheme) { $allowedSchemesSetting[$scheme] = true; } + $defaultScheme = str_starts_with(url('/'), 'http:') ? 'http' : 'https'; + $config->set('URI.SafeIframeRegexp', '%^(http://|https://|//)%'); $config->set('URI.AllowedSchemes', $allowedSchemesSetting); + $config->set('URI.MakeAbsolute', true); + $config->set('URI.DefaultScheme', $defaultScheme); + $config->set('URI.Base', url('/')); // $config->set('Cache.DefinitionImpl', null); // Disable cache during testing } @@ -155,7 +160,8 @@ class ConfiguredHtmlPurifier // Allow mention-ids on links $definition->addAttribute('a', 'data-mention-user-id', 'Number'); - // Set up custom handler for srcset to limit accepted types + // Set up custom handler for srcset + // To remove once added upstream: https://github.com/xemlock/htmlpurifier-html5/pull/91 $definition->addAttribute('img', 'srcset', new SrcsetAttrDef()); $definition->addAttribute('source', 'srcset', new SrcsetAttrDef()); } @@ -163,6 +169,7 @@ class ConfiguredHtmlPurifier protected function configureUriDefinition(HTMLPurifier_URIDefinition $definition): void { $definition->registerFilter(new UriLimitFileProtocolToAnchors()); + $definition->registerFilter(new UriEnsureScheme()); } public function purify(string $html): string diff --git a/app/Util/HtmlPurifier/Filters/UriEnsureScheme.php b/app/Util/HtmlPurifier/Filters/UriEnsureScheme.php new file mode 100644 index 000000000..6b4651b69 --- /dev/null +++ b/app/Util/HtmlPurifier/Filters/UriEnsureScheme.php @@ -0,0 +1,39 @@ +getDefinition('URI'); + $defaultScheme = $def->defaultScheme ?? ''; + + if (empty($uri->scheme) && $defaultScheme) { + $uri->scheme = $defaultScheme; + } + + return true; + } +} diff --git a/app/Util/HtmlPurifier/SrcsetAttrDef.php b/app/Util/HtmlPurifier/SrcsetAttrDef.php index 3b8417a55..2f0ffffab 100644 --- a/app/Util/HtmlPurifier/SrcsetAttrDef.php +++ b/app/Util/HtmlPurifier/SrcsetAttrDef.php @@ -3,24 +3,209 @@ namespace BookStack\Util\HtmlPurifier; use HTMLPurifier_AttrDef; +use HTMLPurifier_AttrDef_URI; /** - * Custom attribute definition to filter out potentially dangerous - * values from the srcset attribute. + * A custom attribute definition for handling Srcset attributes. + * Has been provided upstream via: + * https://github.com/xemlock/htmlpurifier-html5/pull/91 + * but awaiting review/merge. */ class SrcsetAttrDef extends HTMLPurifier_AttrDef { public function validate($string, $config, $context) { - $lower = strtolower($string); - $nonAllowed = ['javascript:', 'vbscript:', 'data:', 'file:']; + $sources = $this->parseImageSources($string); + if (empty($sources)) { + return false; + } - foreach ($nonAllowed as $nonAllowedString) { - if (str_contains($lower, $nonAllowedString)) { - return false; + $uriFilter = new HTMLPurifier_AttrDef_URI(true); + + $filtered = array(); + foreach ($sources as $source) { + $uri = $source['uri']; + $descriptor = $source['descriptor']; + $validatedUri = $uriFilter->validate($uri, $config, $context); + if (is_string($validatedUri)) { + if ($descriptor) { + $filtered[] = $validatedUri . ' ' . $source['descriptor']; + } else { + $filtered[] = $validatedUri; + } } } - return $string; + if (empty($filtered)) { + return false; + } + + return implode(', ', $filtered); + } + + /** + * Parse the image source from srcset attribute text. + * Returns false if it's found to be invalid, otherwise + * returns an array of uri and descriptor combinations. + * + * This aims to follow the WHATWG parsing spec as per: + * https://html.spec.whatwg.org/multipage/images.html#parsing-a-srcset-attribute + * + * @param string $string + * @return array{uri: string, descriptor: string}[]|false + */ + private function parseImageSources($string) + { + $imageSources = array(); + $asciiWhitespace = " \n\r\t\f"; + $asciiWhiteSpaceComma = $asciiWhitespace . ','; + $input = trim($string, $asciiWhiteSpaceComma); + + if ($input === "") { + return false; + } + + $position = 0; + while ($position < strlen($input)) { + $position += strspn($input, $asciiWhitespace, $position); + $urlEnd = $position + strcspn($input, $asciiWhitespace, $position); + $url = substr($input, $position, $urlEnd - $position); + $position = $urlEnd; + $descriptors = array(); + + if (strpos($url, ',') === strlen($url) - 1) { + $url = rtrim($url, ','); + } else { + $position += strspn($input, $asciiWhitespace, $position); + $currentDescriptor = ''; + $state = 'in_descriptor'; + while (true) { + if ($position < strlen($input)) { + $c = $input[$position]; + } else { + $c = null; + } + + if ($state === 'in_descriptor') { + if ($c !== null && strpos($asciiWhitespace, $c) !== false) { + if ($currentDescriptor !== '') { + $descriptors[] = $currentDescriptor; + } + $state = 'after_descriptor'; + } else if ($c === ',') { + $position++; + if ($currentDescriptor !== '') { + $descriptors[] = $currentDescriptor; + } + break; + } else if ($c === '(') { + $currentDescriptor .= $c; + $state = 'in_parens'; + } else if ($c === null) { + if ($currentDescriptor !== '') { + $descriptors[] = $currentDescriptor; + } + break; + } else { + $currentDescriptor .= $c; + } + } else if ($state === 'in_parens') { + if ($c === ')') { + $currentDescriptor .= $c; + $state = 'in_descriptor'; + } else if ($c === null) { + $descriptors[] = $currentDescriptor; + break; + } else { + $currentDescriptor .= $c; + } + } else if ($state === 'after_descriptor') { + if ($c !== null && strpos($asciiWhitespace, $c) !== false) { + // Stay in this state + } else if ($c === null) { + break; + } else { + $state = 'in_descriptor'; + $position--; + } + } + + $position++; + } + } + + $descriptor = $this->formatDescriptor($descriptors); + + if ($url && $descriptor !== false) { + $imageSources[] = array( + 'uri' => $url, + 'descriptor' => $descriptor, + ); + } + } + + return $imageSources; + } + + /** + * Parse and format a single descriptor from an array of potential + * descriptor strings. Returns empty if valid but no descriptor. + * Returns false if invalid. + * @param string[] $descriptors + * @return false|string + */ + private function formatDescriptor(array $descriptors) + { + $error = false; + $width = ''; + $density = ''; + $futureCompatH = ''; + + foreach ($descriptors as $descriptor) { + $descriptor = trim($descriptor); + if ($descriptor === '') { + continue; + } + + $unit = $descriptor[strlen($descriptor) - 1]; + $number = trim(substr($descriptor, 0, -1)); + + if ($unit === 'w' && filter_var($number, FILTER_VALIDATE_INT) && intval($number) >= 0) { + if (!empty($width) || !empty($density) || intval($number) === 0) { + $error = true; + } + $width = $number; + } else if ($unit === 'x' && filter_var($number, FILTER_VALIDATE_FLOAT)) { + if (!empty($width) || !empty($density) || !empty($futureCompatH) || floatval($number) < 0) { + $error = true; + } + $density = $number; + } else if ($unit === 'h' && filter_var($number, FILTER_VALIDATE_INT) && intval($number) >= 0) { + if (!empty($futureCompatH) || !empty($density)) { + $error = true; + } + $futureCompatH = $number; + } else { + $error = true; + } + } + + if (!empty($futureCompatH) && empty($width)) { + $error = true; + } + + if ($error) { + return false; + } + + if ($width) { + return $width . 'w'; + } + + if ($density) { + return $density . 'x'; + } + + return ''; } } diff --git a/tests/Entity/PageContentFilteringTest.php b/tests/Entity/PageContentFilteringTest.php index 5cb898e05..68b597387 100644 --- a/tests/Entity/PageContentFilteringTest.php +++ b/tests/Entity/PageContentFilteringTest.php @@ -488,6 +488,35 @@ HTML; } } + public function test_media_protocol_relative_urls_are_given_scheme_depending_on_app_url() + { + $testCasesExpectedByInput = [ + '