From bc6dea2f8ad1cf0aee0eaa93151332fbac7fb771 Mon Sep 17 00:00:00 2001 From: Yassine Doghri <yassine@doghri.fr> Date: Fri, 25 Mar 2022 14:37:14 +0000 Subject: [PATCH] fix: remove value escaping for form inputs and textareas --- app/Controllers/EpisodeController.php | 2 +- app/Helpers/form_helper.php | 43 +++++++++++++++++++ app/Helpers/rss_helper.php | 12 ++---- app/Helpers/seo_helper.php | 22 +++++----- app/Resources/js/modules/markdown-preview.ts | 11 ++++- app/Views/Components/Forms/FormComponent.php | 5 +++ app/Views/Components/Forms/MarkdownEditor.php | 8 +--- themes/cp_admin/episode/create.php | 2 +- themes/cp_admin/episode/edit.php | 4 +- themes/cp_admin/episode/embed.php | 2 +- themes/cp_admin/episode/publish_edit.php | 2 +- themes/cp_admin/episode/video_clips_new.php | 2 +- themes/cp_admin/page/edit.php | 2 +- themes/cp_admin/podcast/edit.php | 2 +- themes/cp_admin/settings/theme.php | 2 +- themes/cp_app/embed.php | 2 +- themes/cp_app/episode/activity.php | 2 +- themes/cp_auth/reset.php | 2 +- 18 files changed, 86 insertions(+), 41 deletions(-) diff --git a/app/Controllers/EpisodeController.php b/app/Controllers/EpisodeController.php index b4c574497f..f2642152dd 100644 --- a/app/Controllers/EpisodeController.php +++ b/app/Controllers/EpisodeController.php @@ -235,7 +235,7 @@ class EpisodeController extends BaseController $oembed->addChild('thumbnail_height', (string) config('Images')->podcastCoverSizes['og']['height']); $oembed->addChild( 'html', - htmlentities( + htmlspecialchars( '<iframe src="' . $this->episode->embed_url . '" width="100%" height="' . config('Embed')->height . '" frameborder="0" scrolling="no"></iframe>', diff --git a/app/Helpers/form_helper.php b/app/Helpers/form_helper.php index aedc6db3db..bddd4365ae 100644 --- a/app/Helpers/form_helper.php +++ b/app/Helpers/form_helper.php @@ -43,3 +43,46 @@ if (! function_exists('form_markdown_textarea')) { . "</textarea>\n"; } } + + +if (! function_exists('parse_form_attributes')) { + /** + * Parse the form attributes + * + * Helper function used by some of the form helpers + * + * @param array<string, string>|string $attributes List of attributes + * @param array<string, mixed> $default Default values + */ + function parse_form_attributes(array|string $attributes, array $default): string + { + if (is_array($attributes)) { + foreach (array_keys($default) as $key) { + if (isset($attributes[$key])) { + $default[$key] = $attributes[$key]; + unset($attributes[$key]); + } + } + + if (! empty($attributes)) { + $default = array_merge($default, $attributes); + } + } + + $att = ''; + + foreach ($default as $key => $val) { + if (! is_bool($val)) { + if ($key === 'name' && ! strlen($default['name'])) { + continue; + } + + $att .= $key . '="' . $val . '"' . ($key === array_key_last($default) ? '' : ' '); + } else { + $att .= $key . ' '; + } + } + + return $att; + } +} diff --git a/app/Helpers/rss_helper.php b/app/Helpers/rss_helper.php index 075112decb..e40930757d 100644 --- a/app/Helpers/rss_helper.php +++ b/app/Helpers/rss_helper.php @@ -364,22 +364,16 @@ if (! function_exists('get_rss_feed')) { foreach ($episode->persons as $person) { foreach ($person->roles as $role) { - $personElement = $item->addChild( - 'person', - htmlspecialchars($person->full_name), - $podcastNamespace, - ); + $personElement = $item->addChild('person', esc($person->full_name), $podcastNamespace,); $personElement->addAttribute( 'role', - htmlspecialchars( - lang("PersonsTaxonomy.persons.{$role->group}.roles.{$role->role}.label", [], 'en'), - ), + esc(lang("PersonsTaxonomy.persons.{$role->group}.roles.{$role->role}.label", [], 'en'),), ); $personElement->addAttribute( 'group', - htmlspecialchars(lang("PersonsTaxonomy.persons.{$role->group}.label", [], 'en')), + esc(lang("PersonsTaxonomy.persons.{$role->group}.label", [], 'en')), ); $personElement->addAttribute('img', $person->avatar->medium_url); diff --git a/app/Helpers/seo_helper.php b/app/Helpers/seo_helper.php index 284af09398..473df8adb2 100644 --- a/app/Helpers/seo_helper.php +++ b/app/Helpers/seo_helper.php @@ -30,8 +30,8 @@ if (! function_exists('get_podcast_metatags')) { $schema = new Schema( new Thing('PodcastSeries', [ - 'name' => esc($podcast->title), - 'headline' => esc($podcast->title), + 'name' => $podcast->title, + 'headline' => $podcast->title, 'url' => current_url(), 'sameAs' => $podcast->link, 'identifier' => $podcast->guid, @@ -39,8 +39,8 @@ if (! function_exists('get_podcast_metatags')) { 'description' => $podcast->description, 'webFeed' => $podcast->feed_url, 'accessMode' => 'auditory', - 'author' => esc($podcast->owner_name), - 'creator' => esc($podcast->owner_name), + 'author' => $podcast->owner_name, + 'creator' => $podcast->owner_name, 'publisher' => $podcast->publisher, 'inLanguage' => $podcast->language_code, 'genre' => $category, @@ -50,8 +50,8 @@ if (! function_exists('get_podcast_metatags')) { $metatags = new MetaTags(); $metatags - ->title(esc($podcast->title) . ' (@' . esc($podcast->handle) . ') • ' . lang('Podcast.' . $page)) - ->description(htmlspecialchars($podcast->description)) + ->title($podcast->title . ' (@' . $podcast->handle . ') • ' . lang('Podcast.' . $page)) + ->description(esc($podcast->description)) ->image((string) $podcast->cover->og_url) ->canonical((string) current_url()) ->og('image:width', (string) config('Images')->podcastCoverSizes['og']['width']) @@ -80,7 +80,7 @@ if (! function_exists('get_episode_metatags')) { $schema = new Schema( new Thing('PodcastEpisode', [ 'url' => url_to('episode', esc($episode->podcast->handle), $episode->slug), - 'name' => esc($episode->title), + 'name' => $episode->title, 'image' => $episode->cover->feed_url, 'description' => $episode->description, 'datePublished' => $episode->published_at->format(DATE_ISO8601), @@ -90,7 +90,7 @@ if (! function_exists('get_episode_metatags')) { 'contentUrl' => $episode->audio->file_url, ]), 'partOfSeries' => new Thing('PodcastSeries', [ - 'name' => esc($episode->podcast->title), + 'name' => $episode->podcast->title, 'url' => $episode->podcast->link, ]), ]) @@ -271,7 +271,7 @@ if (! function_exists('get_home_metatags')) { { $metatags = new MetaTags(); $metatags - ->title(esc(service('settings')->get('App.siteName'))) + ->title(service('settings')->get('App.siteName')) ->description(esc(service('settings')->get('App.siteDescription'))) ->image(service('settings')->get('App.siteIcon')['512']) ->canonical((string) current_url()) @@ -287,9 +287,9 @@ if (! function_exists('get_page_metatags')) { $metatags = new MetaTags(); $metatags ->title( - esc($page->title) . service('settings')->get('App.siteTitleSeparator') . esc(service( + $page->title . service('settings')->get('App.siteTitleSeparator') . service( 'settings' - )->get('App.siteName')) + )->get('App.siteName') ) ->description(esc(service('settings')->get('App.siteDescription'))) ->image(service('settings')->get('App.siteIcon')['512']) diff --git a/app/Resources/js/modules/markdown-preview.ts b/app/Resources/js/modules/markdown-preview.ts index ed0c5a91bd..58733f7bf8 100644 --- a/app/Resources/js/modules/markdown-preview.ts +++ b/app/Resources/js/modules/markdown-preview.ts @@ -47,11 +47,20 @@ export class MarkdownPreview extends LitElement { return link.replace("<a", "<a target='_blank' rel='noopener noreferrer'"); }; - return marked(this._textarea.value, { + return marked(this.escapeHtml(this._textarea.value), { renderer: renderer, }); } + private escapeHtml = (unsafe: string) => { + return unsafe + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">") + .replaceAll('"', """) + .replaceAll("'", "'"); + }; + static styles = css` * { max-width: 65ch; diff --git a/app/Views/Components/Forms/FormComponent.php b/app/Views/Components/Forms/FormComponent.php index 941f0814c1..d8883e0483 100644 --- a/app/Views/Components/Forms/FormComponent.php +++ b/app/Views/Components/Forms/FormComponent.php @@ -31,6 +31,11 @@ class FormComponent extends Component } } + public function setValue(string $value): void + { + $this->value = htmlspecialchars_decode($value, ENT_QUOTES); + } + public function setRequired(string $value): void { $this->required = $value === 'true'; diff --git a/app/Views/Components/Forms/MarkdownEditor.php b/app/Views/Components/Forms/MarkdownEditor.php index 66fb54c277..617c05a221 100644 --- a/app/Views/Components/Forms/MarkdownEditor.php +++ b/app/Views/Components/Forms/MarkdownEditor.php @@ -23,13 +23,7 @@ class MarkdownEditor extends FormComponent $this->attributes['class'] = 'bg-elevated border-none focus:border-none focus:outline-none focus:ring-0 w-full h-full'; $this->attributes['rows'] = 6; - $value = htmlspecialchars_decode($this->value); - - $oldValue = old($this->name); - if ($oldValue === null) { - $oldValue = $value; - } - $textarea = form_textarea($this->attributes, $oldValue); + $textarea = form_markdown_textarea($this->attributes, old($this->name, $this->value)); $markdownIcon = icon( 'markdown', 'mr-1 text-lg opacity-40' diff --git a/themes/cp_admin/episode/create.php b/themes/cp_admin/episode/create.php index a327ff86e3..61f050730b 100644 --- a/themes/cp_admin/episode/create.php +++ b/themes/cp_admin/episode/create.php @@ -126,7 +126,7 @@ name="description_footer" label="<?= lang('Episode.form.description_footer') ?>" hint="<?= lang('Episode.form.description_footer_hint') ?>" - value="<?= htmlspecialchars($podcast->episode_description_footer_markdown) ?? '' ?>" + value="<?= esc($podcast->episode_description_footer_markdown) ?? '' ?>" disallowList="header,quote" /> </Forms.Section> diff --git a/themes/cp_admin/episode/edit.php b/themes/cp_admin/episode/edit.php index 70453ca31a..5611b60b0a 100644 --- a/themes/cp_admin/episode/edit.php +++ b/themes/cp_admin/episode/edit.php @@ -122,7 +122,7 @@ as="MarkdownEditor" name="description" label="<?= lang('Episode.form.description') ?>" - value="<?= htmlspecialchars($episode->description_markdown) ?>" + value="<?= esc($episode->description_markdown) ?>" required="true" disallowList="header,quote" /> @@ -131,7 +131,7 @@ name="description_footer" label="<?= lang('Episode.form.description_footer') ?>" hint="<?= lang('Episode.form.description_footer_hint') ?>" - value="<?= htmlspecialchars($podcast->episode_description_footer_markdown) ?? '' ?>" + value="<?= esc($podcast->episode_description_footer_markdown) ?? '' ?>" disallowList="header,quote" /> </Forms.Section> diff --git a/themes/cp_admin/episode/embed.php b/themes/cp_admin/episode/embed.php index 16057d97f2..f633a04a60 100644 --- a/themes/cp_admin/episode/embed.php +++ b/themes/cp_admin/episode/embed.php @@ -32,7 +32,7 @@ </div> <div class="flex items-center mt-4 gap-x-2"> - <Forms.Input readonly="true" class="w-full max-w-xl" name="url" value="<?= $episode->embed_url ?>" /> + <Forms.Input readonly="true" class="w-full max-w-xl" name="url" value="<?= esc($episode->embed_url) ?>" /> <IconButton glyph="file-copy" data-type="clipboard-copy" data-clipboard-target="url"><?= lang('Episode.embed.clipboard_url') ?></IconButton> </div> diff --git a/themes/cp_admin/episode/publish_edit.php b/themes/cp_admin/episode/publish_edit.php index 7144875343..56af8dfd4e 100644 --- a/themes/cp_admin/episode/publish_edit.php +++ b/themes/cp_admin/episode/publish_edit.php @@ -39,7 +39,7 @@ </div> </div> <div class="px-4 mb-2"> - <Forms.Textarea name="message" placeholder="<?= lang('Episode.publish_form.message_placeholder') ?>" autofocus="" value="<?= $post->message ?>" rows="2" /> + <Forms.Textarea name="message" placeholder="<?= lang('Episode.publish_form.message_placeholder') ?>" autofocus="" value="<?= esc($post->message) ?>" rows="2" /> </div> <div class="flex border-y"> <img src="<?= $episode->cover diff --git a/themes/cp_admin/episode/video_clips_new.php b/themes/cp_admin/episode/video_clips_new.php index 7b7f2839d0..3f3ca52a27 100644 --- a/themes/cp_admin/episode/video_clips_new.php +++ b/themes/cp_admin/episode/video_clips_new.php @@ -58,7 +58,7 @@ <?php foreach (config('MediaClipper')->themes as $themeName => $colors): ?> <Forms.ColorRadioButton class="mx-auto" - value="<?= $themeName ?>" + value="<?= esc($themeName) ?>" name="theme" required="true" isChecked="<?= $themeName === 'pine' ? 'true' : 'false' ?>" diff --git a/themes/cp_admin/page/edit.php b/themes/cp_admin/page/edit.php index 9e051f75c5..e1864e210e 100644 --- a/themes/cp_admin/page/edit.php +++ b/themes/cp_admin/page/edit.php @@ -35,7 +35,7 @@ as="MarkdownEditor" name="content" label="<?= lang('Page.form.content') ?>" - value="<?= htmlspecialchars($page->content_markdown) ?>" + value="<?= esc($page->content_markdown) ?>" required="true" rows="20" /> diff --git a/themes/cp_admin/podcast/edit.php b/themes/cp_admin/podcast/edit.php index 661c111148..527e3544df 100644 --- a/themes/cp_admin/podcast/edit.php +++ b/themes/cp_admin/podcast/edit.php @@ -67,7 +67,7 @@ as="MarkdownEditor" name="description" label="<?= lang('Podcast.form.description') ?>" - value="<?= htmlspecialchars($podcast->description_markdown) ?>" + value="<?= esc($podcast->description_markdown) ?>" required="true" disallowList="header,quote" /> diff --git a/themes/cp_admin/settings/theme.php b/themes/cp_admin/settings/theme.php index 93d321f317..4757a21460 100644 --- a/themes/cp_admin/settings/theme.php +++ b/themes/cp_admin/settings/theme.php @@ -20,7 +20,7 @@ <?php foreach (config('Colors')->themes as $themeName => $color): ?> <Forms.ColorRadioButton class="theme-<?= $themeName ?> mx-auto" - value="<?= $themeName ?>" + value="<?= esc($themeName) ?>" name="theme" isChecked="<?= $themeName === service('settings') ->get('App.theme') ? 'true' : 'false' ?>" ><?= lang('Settings.theme.' . $themeName) ?></Forms.ColorRadioButton> diff --git a/themes/cp_app/embed.php b/themes/cp_app/embed.php index 8e858f0bc9..3dc10c6139 100644 --- a/themes/cp_app/embed.php +++ b/themes/cp_app/embed.php @@ -6,7 +6,7 @@ <meta charset="UTF-8" /> <meta name="viewport" content="width=device-width, initial-scale=1.0"/> <title><?= esc($episode->title) ?></title> - <meta name="description" content="<?= htmlspecialchars( + <meta name="description" content="<?= esc( $episode->description, ) ?>" /> <link rel="icon" type="image/x-icon" href="<?= service('settings') diff --git a/themes/cp_app/episode/activity.php b/themes/cp_app/episode/activity.php index 293a8c73e7..f84a0e337e 100644 --- a/themes/cp_app/episode/activity.php +++ b/themes/cp_app/episode/activity.php @@ -11,7 +11,7 @@ ->avatar_image_url ?>" alt="<?= esc(interact_as_actor() ->display_name) ?>" class="w-10 h-10 rounded-full aspect-square" loading="lazy" /> <div class="flex flex-col flex-1 min-w-0 gap-y-2"> - <input name="episode_url" value="<?= $episode->link ?>" type="hidden" /> + <input name="episode_url" value="<?= esc($episode->link) ?>" type="hidden" /> <Forms.Textarea name="message" placeholder="<?= lang('Post.form.episode_message_placeholder') ?>" diff --git a/themes/cp_auth/reset.php b/themes/cp_auth/reset.php index a711430fdd..2b9199a98b 100644 --- a/themes/cp_auth/reset.php +++ b/themes/cp_auth/reset.php @@ -15,7 +15,7 @@ <Forms.Field name="token" label="<?= lang('Auth.token') ?>" - value="<?= $token ?? '' ?>" + value="<?= esc($token) ?? '' ?>" required="true" /> <Forms.Field -- GitLab