Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix request #31586: XSS in the tooltip via an artifact title
Next step for the current story #26777 is to display the xref on top of
the title so in order to prepare the field we use mustache to escape the
title instead of DOMPurifier.

Part of story #26777: have artifact tooltips on roadmap

Change-Id: I534ead8a88361b364f5ee81556251dc3dc4c0bf6
  • Loading branch information
nterray committed Apr 20, 2023
1 parent 20b9cf4 commit fdc93a7
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 11 deletions.
1 change: 1 addition & 0 deletions plugins/tracker/include/Tracker/Artifact/Artifact.php
Expand Up @@ -456,6 +456,7 @@ public function fetchTooltip(PFUser $user): Tuleap\Option\Option
{
$progress_dao = new SemanticProgressDao();
$tooltip_fetcher = new Tuleap\Tracker\Semantic\Tooltip\TooltipFetcher(
TemplateRendererFactory::build(),
new Tuleap\Tracker\Semantic\Tooltip\OtherSemantic\ProgressTooltipEntry(
new SemanticProgressBuilder(
$progress_dao,
Expand Down
Expand Up @@ -33,8 +33,10 @@ final class TooltipFetcher
*/
private readonly array $other_semantic_tooltip_entry_fetchers;

public function __construct(OtherSemanticTooltipEntryFetcher ...$other_semantic_tooltip_entry_fetchers)
{
public function __construct(
private readonly \TemplateRendererFactory $renderer_factory,
OtherSemanticTooltipEntryFetcher ...$other_semantic_tooltip_entry_fetchers,
) {
$this->other_semantic_tooltip_entry_fetchers = $other_semantic_tooltip_entry_fetchers;
}

Expand All @@ -58,8 +60,12 @@ public function fetchArtifactTooltip(Artifact $artifact, TooltipFields $tooltip,
$html .= '</table>';

return Option::fromValue(
TooltipJSON::fromHtmlTitleAndHtmlBody((string) $artifact->getTitle(), $html)
->withAccentColor($artifact->getTracker()->getColor()->getName())
TooltipJSON::fromHtmlTitleAndHtmlBody(
$this->renderer_factory
->getRenderer(__DIR__ . '/../../../../templates/tooltip/')
->renderToString('artifact-tooltip-title', ['title' => $artifact->getTitle()]),
$html
)->withAccentColor($artifact->getTracker()->getColor()->getName())
);
}

Expand Down
@@ -0,0 +1 @@
{{ title }}
Expand Up @@ -22,6 +22,8 @@

namespace Tuleap\Tracker\Semantic\Tooltip;

use TemplateRendererFactory;
use Tuleap\Templating\TemplateCache;
use Tuleap\Test\Builders\UserTestBuilder;
use Tuleap\Test\PHPUnit\TestCase;
use Tuleap\Tracker\Artifact\Artifact;
Expand All @@ -42,8 +44,12 @@ public function testNothingWhenArtifactIsNotReadable(): void

$user = UserTestBuilder::buildWithDefaults();

$template_cache = $this->createMock(TemplateCache::class);
$template_cache->method('getPath')->willReturn(null);
$template_factory = new TemplateRendererFactory($template_cache);

self::assertTrue(
(new TooltipFetcher())
(new TooltipFetcher($template_factory))
->fetchArtifactTooltip($artifact, $tooltip, $user)
->isNothing()
);
Expand All @@ -58,8 +64,12 @@ public function testNothingWhenThereIsNoFields(): void

$user = UserTestBuilder::buildWithDefaults();

$template_cache = $this->createMock(TemplateCache::class);
$template_cache->method('getPath')->willReturn(null);
$template_factory = new TemplateRendererFactory($template_cache);

self::assertTrue(
(new TooltipFetcher())
(new TooltipFetcher($template_factory))
->fetchArtifactTooltip($artifact, $tooltip, $user)
->isNothing()
);
Expand Down Expand Up @@ -91,8 +101,12 @@ public function testReturnTheTooltipValueOfEachFields(): void

$user = UserTestBuilder::buildWithDefaults();

$tooltip = (new TooltipFetcher())->fetchArtifactTooltip($artifact, $tooltip, $user);
self::assertEquals('The title', $tooltip->unwrapOr('')->title_as_html);
$template_cache = $this->createMock(TemplateCache::class);
$template_cache->method('getPath')->willReturn(null);
$template_factory = new TemplateRendererFactory($template_cache);

$tooltip = (new TooltipFetcher($template_factory))->fetchArtifactTooltip($artifact, $tooltip, $user);
self::assertStringContainsString('The title', $tooltip->unwrapOr('')->title_as_html);
self::assertStringContainsString('avada', $tooltip->unwrapOr('')->body_as_html);
self::assertStringContainsString('kedavra', $tooltip->unwrapOr('')->body_as_html);
self::assertEquals('fiesta-red', $tooltip->unwrapOr('')->accent_color);
Expand Down Expand Up @@ -124,7 +138,12 @@ public function testIncludesOtherSemanticsEntries(): void

$user = UserTestBuilder::buildWithDefaults();

$template_cache = $this->createMock(TemplateCache::class);
$template_cache->method('getPath')->willReturn(null);
$template_factory = new TemplateRendererFactory($template_cache);

$tooltip = (new TooltipFetcher(
$template_factory,
new class implements OtherSemanticTooltipEntryFetcher {
public function fetchTooltipEntry(Artifact $artifact, \PFUser $user): string
{
Expand All @@ -138,7 +157,7 @@ public function fetchTooltipEntry(Artifact $artifact, \PFUser $user): string
}
},
))->fetchArtifactTooltip($artifact, $tooltip, $user);
self::assertEquals('The title', $tooltip->unwrapOr('')->title_as_html);
self::assertStringContainsString('The title', $tooltip->unwrapOr('')->title_as_html);
self::assertStringContainsString('Susan', $tooltip->unwrapOr('')->body_as_html);
self::assertStringContainsString('Dennis', $tooltip->unwrapOr('')->body_as_html);
self::assertStringContainsString('avada', $tooltip->unwrapOr('')->body_as_html);
Expand Down Expand Up @@ -168,8 +187,12 @@ public function testExcludeUnreadableFields(): void

$user = UserTestBuilder::buildWithDefaults();

$tooltip = (new TooltipFetcher())->fetchArtifactTooltip($artifact, $tooltip, $user);
self::assertEquals('', $tooltip->unwrapOr('')->title_as_html);
$template_cache = $this->createMock(TemplateCache::class);
$template_cache->method('getPath')->willReturn(null);
$template_factory = new TemplateRendererFactory($template_cache);

$tooltip = (new TooltipFetcher($template_factory))->fetchArtifactTooltip($artifact, $tooltip, $user);
self::assertEquals('', trim($tooltip->unwrapOr('')->title_as_html));
self::assertStringContainsString('avada', $tooltip->unwrapOr('')->body_as_html);
self::assertStringNotContainsString('kedavra', $tooltip->unwrapOr('')->body_as_html);
}
Expand Down

0 comments on commit fdc93a7

Please sign in to comment.