Skip to content

Commit 0e0a17c

Browse files
ssddanbrownssddanbrown
committed
Prevented page text content includes
Avoids possible permission issues where included content shown in search or preview where the user would not normally have permission to view the included content. Closes BookStackApp#1178
1 parent ffceb40 commit 0e0a17c

File tree

3 files changed

+47
-23
lines changed

3 files changed

+47
-23
lines changed

‎app/Entities/Repos/EntityRepo.php

+40-19
Original file line numberDiff line numberDiff line change
@@ -599,24 +599,48 @@ protected function nameToSlug($name)
599599
}
600600

601601
/**
602-
* Render the page for viewing, Parsing and performing features such as page transclusion.
602+
* Render the page for viewing
603603
* @param Page $page
604-
* @param bool $ignorePermissions
605-
* @return mixed|string
604+
* @param bool $blankIncludes
605+
* @return string
606606
*/
607-
public function renderPage(Page $page, $ignorePermissions = false)
607+
public function renderPage(Page $page, bool $blankIncludes = false) : string
608608
{
609609
$content = $page->html;
610+
610611
if (!config('app.allow_content_scripts')) {
611612
$content = $this->escapeScripts($content);
612613
}
613614

614-
$matches = [];
615-
preg_match_all("/{{@\s?([0-9].*?)}}/", $content, $matches);
616-
if (count($matches[0]) === 0) {
617-
return $content;
615+
if ($blankIncludes) {
616+
$content = $this->blankPageIncludes($content);
617+
} else {
618+
$content = $this->parsePageIncludes($content);
618619
}
619620

621+
return $content;
622+
}
623+
624+
/**
625+
* Remove any page include tags within the given HTML.
626+
* @param string $html
627+
* @return string
628+
*/
629+
protected function blankPageIncludes(string $html) : string
630+
{
631+
return preg_replace("/{{@\s?([0-9].*?)}}/", '', $html);
632+
}
633+
634+
/**
635+
* Parse any include tags "{{@<page_id>#section}}" to be part of the page.
636+
* @param string $html
637+
* @return mixed|string
638+
*/
639+
protected function parsePageIncludes(string $html) : string
640+
{
641+
$matches = [];
642+
preg_match_all("/{{@\s?([0-9].*?)}}/", $html, $matches);
643+
620644
$topLevelTags = ['table', 'ul', 'ol'];
621645
foreach ($matches[1] as $index => $includeId) {
622646
$splitInclude = explode('#', $includeId, 2);
@@ -625,22 +649,22 @@ public function renderPage(Page $page, $ignorePermissions = false)
625649
continue;
626650
}
627651

628-
$matchedPage = $this->getById('page', $pageId, false, $ignorePermissions);
652+
$matchedPage = $this->getById('page', $pageId);
629653
if ($matchedPage === null) {
630-
$content = str_replace($matches[0][$index], '', $content);
654+
$html = str_replace($matches[0][$index], '', $html);
631655
continue;
632656
}
633657

634658
if (count($splitInclude) === 1) {
635-
$content = str_replace($matches[0][$index], $matchedPage->html, $content);
659+
$html = str_replace($matches[0][$index], $matchedPage->html, $html);
636660
continue;
637661
}
638662

639663
$doc = new DOMDocument();
640664
$doc->loadHTML(mb_convert_encoding('<body>'.$matchedPage->html.'</body>', 'HTML-ENTITIES', 'UTF-8'));
641665
$matchingElem = $doc->getElementById($splitInclude[1]);
642666
if ($matchingElem === null) {
643-
$content = str_replace($matches[0][$index], '', $content);
667+
$html = str_replace($matches[0][$index], '', $html);
644668
continue;
645669
}
646670
$innerContent = '';
@@ -652,25 +676,22 @@ public function renderPage(Page $page, $ignorePermissions = false)
652676
$innerContent .= $doc->saveHTML($childNode);
653677
}
654678
}
655-
$content = str_replace($matches[0][$index], trim($innerContent), $content);
679+
$html = str_replace($matches[0][$index], trim($innerContent), $html);
656680
}
657681

658-
return $content;
682+
return $html;
659683
}
660684

661685
/**
662686
* Escape script tags within HTML content.
663687
* @param string $html
664-
* @return mixed
688+
* @return string
665689
*/
666-
protected function escapeScripts(string $html)
690+
protected function escapeScripts(string $html) : string
667691
{
668692
$scriptSearchRegex = '/<script.*?>.*?<\/script>/ms';
669693
$matches = [];
670694
preg_match_all($scriptSearchRegex, $html, $matches);
671-
if (count($matches) === 0) {
672-
return $html;
673-
}
674695

675696
foreach ($matches[0] as $match) {
676697
$html = str_replace($match, htmlentities($match), $html);

‎app/Entities/Repos/PageRepo.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ protected function formatHtml(string $htmlText)
194194
* @param \BookStack\Entities\Page $page
195195
* @return string
196196
*/
197-
public function pageToPlainText(Page $page)
197+
protected function pageToPlainText(Page $page) : string
198198
{
199-
$html = $this->renderPage($page);
199+
$html = $this->renderPage($page, true);
200200
return strip_tags($html);
201201
}
202202

‎tests/Entity/PageContentTest.php

+5-2
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,18 @@ public function test_saving_page_with_includes()
4040
{
4141
$page = Page::first();
4242
$secondPage = Page::where('id', '!=', $page->id)->first();
43+
4344
$this->asEditor();
44-
$page->html = "<p>{{@$secondPage->id}}</p>";
45+
$includeTag = '{{@' . $secondPage->id . '}}';
46+
$page->html = '<p>' . $includeTag . '</p>';
4547

4648
$resp = $this->put($page->getUrl(), ['name' => $page->name, 'html' => $page->html, 'summary' => '']);
4749

4850
$resp->assertStatus(302);
4951

5052
$page = Page::find($page->id);
51-
$this->assertContains("{{@$secondPage->id}}", $page->html);
53+
$this->assertContains($includeTag, $page->html);
54+
$this->assertEquals('', $page->text);
5255
}
5356

5457
public function test_page_includes_do_not_break_tables()

0 commit comments

Comments
 (0)