From ab0f53ba262c1a72d5a417b47a0b1b82a39118e8 Mon Sep 17 00:00:00 2001 From: swag Date: Wed, 3 May 2023 21:05:57 -0400 Subject: [PATCH] Sanitize HTML in body; enforce NOT NULL on body; template clean-up --- README.md | 3 ++- cpanfile | 1 + lib/PostText.pm | 17 ++++++++++++++--- lib/PostText/Model/Remark.pm | 5 +++-- lib/PostText/Model/Thread.pm | 5 +++-- migrations/12/down.sql | 7 +++++++ migrations/12/up.sql | 9 +++++++++ templates/page/about.html.ep | 21 +++++++++++---------- templates/remark/by_id.html.ep | 4 +++- templates/remark/create.html.ep | 8 ++++++-- templates/thread/by_id.html.ep | 8 ++++++-- templates/thread/by_page.html.ep | 4 +++- 12 files changed, 68 insertions(+), 24 deletions(-) create mode 100644 migrations/12/down.sql create mode 100644 migrations/12/up.sql diff --git a/README.md b/README.md index 7ad60cd..815bacc 100644 --- a/README.md +++ b/README.md @@ -58,9 +58,10 @@ Run the tests locally (against development environment): ## TODOs -1. Use something like HTML::Restrict on thread/remark body 1. CSS 1. "All new posts flagged" mode (require approval for new posts) +1. Tests for mod-only user? +1. Check input validation ## Crazy future ideas diff --git a/cpanfile b/cpanfile index 5c43db3..4cf0e75 100644 --- a/cpanfile +++ b/cpanfile @@ -9,3 +9,4 @@ requires 'Date::Format'; requires 'XML::RSS'; requires 'CSS::Minifier::XS'; requires 'Text::Markdown'; +requires 'HTML::Restrict'; diff --git a/lib/PostText.pm b/lib/PostText.pm index 9e1eeb0..8ec277f 100644 --- a/lib/PostText.pm +++ b/lib/PostText.pm @@ -6,6 +6,7 @@ use Mojo::Base 'Mojolicious', -signatures; use Mojo::Pg; use Crypt::Passphrase; use Text::Markdown qw{markdown}; +use HTML::Restrict; # The local libs use PostText::Model::Thread; @@ -29,12 +30,22 @@ sub startup($self) { ) }); + $self->helper(hr => sub ($c) { + state $hr = HTML::Restrict->new(strip_enclosed_content => []) + }); + $self->helper(thread => sub ($c) { - state $thread = PostText::Model::Thread->new(pg => $c->pg) + state $thread = PostText::Model::Thread->new( + pg => $c->pg, + hr => $c->hr + ) }); $self->helper(remark => sub ($c) { - state $remark = PostText::Model::Remark->new(pg => $c->pg) + state $remark = PostText::Model::Remark->new( + pg => $c->pg, + hr => $c->hr + ) }); $self->helper(moderator => sub ($c) { @@ -70,7 +81,7 @@ sub startup($self) { # Finish configuring some things $self->secrets($self->config->{'secrets'}) || die $@; - $self->pg->migrations->from_dir('migrations')->migrate(11); + $self->pg->migrations->from_dir('migrations')->migrate(12); if (my $threads_per_page = $self->config->{'threads_per_page'}) { $self->thread->per_page($threads_per_page) diff --git a/lib/PostText/Model/Remark.pm b/lib/PostText/Model/Remark.pm index 69fe6ac..4bf0485 100644 --- a/lib/PostText/Model/Remark.pm +++ b/lib/PostText/Model/Remark.pm @@ -2,7 +2,7 @@ package PostText::Model::Remark; use Mojo::Base -base, -signatures; -has 'pg'; +has [qw{pg hr}]; has per_page => 5; @@ -28,7 +28,8 @@ sub by_page_for($self, $thread_id, $this_page = 1) { } sub create($self, $thread_id, $author, $body, $hidden = 0, $flagged = 0) { - my @data = ($thread_id, $author, $body, $hidden, $flagged); + my $clean_body = $self->hr->process($body); + my @data = ($thread_id, $author, $clean_body, $hidden, $flagged); $self->pg->db->query(<<~'END_SQL', @data); INSERT INTO remarks ( diff --git a/lib/PostText/Model/Thread.pm b/lib/PostText/Model/Thread.pm index cf2f0c1..a81b7e8 100644 --- a/lib/PostText/Model/Thread.pm +++ b/lib/PostText/Model/Thread.pm @@ -2,14 +2,15 @@ package PostText::Model::Thread; use Mojo::Base -base, -signatures; -has 'pg'; +has [qw{pg hr}]; has per_page => 5; has date_format => 'Dy, FMDD Mon YYYY HH24:MI:SS TZ'; sub create($self, $author, $title, $body, $hidden = 0, $flagged = 0) { - my @data = ($author, $title, $body, $hidden, $flagged); + my $clean_body = $self->hr->process($body); + my @data = ($author, $title, $clean_body, $hidden, $flagged); $self->pg->db->query(<<~'END_SQL', @data)->hash->{'thread_id'}; INSERT INTO threads ( diff --git a/migrations/12/down.sql b/migrations/12/down.sql new file mode 100644 index 0000000..bad6667 --- /dev/null +++ b/migrations/12/down.sql @@ -0,0 +1,7 @@ +ALTER TABLE threads +ALTER COLUMN thread_body + DROP NOT NULL; + +ALTER TABLE remarks +ALTER COLUMN remark_body + DROP NOT NULL; diff --git a/migrations/12/up.sql b/migrations/12/up.sql new file mode 100644 index 0000000..8d42d40 --- /dev/null +++ b/migrations/12/up.sql @@ -0,0 +1,9 @@ +-- Input validation may fail for body if user submits only an HTML tag + +ALTER TABLE threads +ALTER COLUMN thread_body + SET NOT NULL; + +ALTER TABLE remarks +ALTER COLUMN remark_body + SET NOT NULL; diff --git a/templates/page/about.html.ep b/templates/page/about.html.ep index 1f77b7d..9185688 100644 --- a/templates/page/about.html.ep +++ b/templates/page/about.html.ep @@ -3,13 +3,13 @@

<%= title %>

Post::Text is a textboard a bit like 2channel. You - can post whatever you want anonymously just please mind the - <%= link_to rules => 'rules_page' %>. Markdown is supported for - formatting posts using the - - original implementation from The Daring Fireball. For - example, back-ticks are for inline code while - indentation should be used if you want an entire code bock:

+ can post whatever you want anonymously just please mind the <%= + link_to rules => 'rules_page' %>. Markdown is supported for + formatting posts using the + original implementation from The Daring Fireball. For example, + back-ticks are for inline code while indentation should + be used if you want an entire code bock:


 This is `inline_code()` and so is ```this()```. This is incorrect:
 
@@ -24,10 +24,11 @@ This is correct for a multi-line code block:
     if (true) {
         do_stuff();
     }
-
-You can use an actual tab character or four spaces to indent.
-

There is a button for users to 'flag' a post for review by a +

You can use an actual tab character or four spaces to indent. + Only Markdown is supported, HTML will be filtered + out.

+

There is a button for users to 'flag' a post for review by a moderator. If you need further assistance you can reach out to the webmaster. There is also a 'bump' button you're free to use and abuse to your heart's diff --git a/templates/remark/by_id.html.ep b/templates/remark/by_id.html.ep index 09ced38..22e9cbf 100644 --- a/templates/remark/by_id.html.ep +++ b/templates/remark/by_id.html.ep @@ -6,7 +6,9 @@

<%= $remark->{'date'} %>

<%= $remark->{'author'} %>
-
<%== markdown $remark->{'body'} %>
+
+ <%== markdown $remark->{'body'} =%> +

<%= $last_remark->{'date'} %>

<%= $last_remark->{'author'} %>
-
<%== markdown $last_remark->{'body'} %>
+
+ <%== markdown $last_remark->{'body'} =%> +
<% } =%> diff --git a/templates/thread/by_id.html.ep b/templates/thread/by_id.html.ep index a6d486e..1a3a5ce 100644 --- a/templates/thread/by_id.html.ep +++ b/templates/thread/by_id.html.ep @@ -6,7 +6,9 @@

<%= $thread->{'title'} %>

<%= $thread->{'date'} %>

<%= $thread->{'author'} %>
-
<%== markdown $thread->{'body'} %>
+
+ <%== markdown $thread->{'body'} =%> +

<%= $remark->{'date'} %>

<%= $remark->{'author'} %>
-
<%== markdown $remark->{'body'} %>
+
+ <%== markdown $remark->{'body'} =%> +
diff --git a/templates/thread/by_page.html.ep b/templates/thread/by_page.html.ep index f83d7be..067f7b3 100644 --- a/templates/thread/by_page.html.ep +++ b/templates/thread/by_page.html.ep @@ -15,7 +15,9 @@

<%= $thread->{'date'} %>

<%= $thread->{'author'} %>
-
<%== markdown truncate_text $thread->{'body'} %>
+
+ <%== markdown truncate_text $thread->{'body'} =%> +