diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | cpanfile | 1 | ||||
| -rw-r--r-- | cpanfile.snapshot | 13 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 6 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin/Bodies.pm | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Template.pm | 17 | ||||
| -rw-r--r-- | perllib/Open311/PopulateServiceList.pm | 8 | ||||
| -rw-r--r-- | t/app/controller/admin/bodies.t | 4 | ||||
| -rw-r--r-- | t/app/controller/admin/reportextrafields.t | 2 | ||||
| -rw-r--r-- | t/open311/populate-service-list.t | 4 |
10 files changed, 49 insertions, 8 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c7a46b9..4f475bfa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Fix XSS vulnerability in pagination page number. - Rotate session ID after successful login. - Switch to auto-escaping of all template variables (see below). + - Scrub admin description fields. - Front end improvements: - Improved 403 message, especially for private reports. #2511 - Mobile users can now filter the pins on the `/around` map view. #2366 @@ -85,6 +85,7 @@ requires 'Geography::NationalGrid', requires 'Getopt::Long::Descriptive'; requires 'HTML::Entities'; requires 'HTML::FormHandler::Model::DBIC'; +requires 'HTML::Scrubber'; requires 'HTTP::Request::Common'; requires 'Image::Size', '3.300'; requires 'Image::PNG::QRCode'; diff --git a/cpanfile.snapshot b/cpanfile.snapshot index 85132bbb6..42c125da6 100644 --- a/cpanfile.snapshot +++ b/cpanfile.snapshot @@ -3377,6 +3377,19 @@ DISTRIBUTIONS HTML::Tagset 3 XSLoader 0 perl 5.008 + HTML-Scrubber-0.19 + pathname: N/NI/NIGELM/HTML-Scrubber-0.19.tar.gz + provides: + HTML::Scrubber 0.19 + requirements: + ExtUtils::MakeMaker 0 + HTML::Entities 0 + HTML::Parser 3.47 + List::Util 1.33 + Scalar::Util 0 + perl 5.008 + strict 0 + warnings 0 HTML-Selector-XPath-0.15 pathname: C/CO/CORION/HTML-Selector-XPath-0.15.tar.gz provides: diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 64cc9eaaf..c1afccdfd 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -557,7 +557,8 @@ sub update_extra_fields : Private { if ($behaviour eq 'question') { $meta->{required} = $c->get_param("metadata[$i].required") ? 'true' : 'false'; $meta->{variable} = 'true'; - $meta->{description} = $c->get_param("metadata[$i].description"); + my $desc = $c->get_param("metadata[$i].description"); + $meta->{description} = FixMyStreet::Template::sanitize($desc); $meta->{datatype} = $c->get_param("metadata[$i].datatype"); if ( $meta->{datatype} eq "singlevaluelist" ) { @@ -579,7 +580,8 @@ sub update_extra_fields : Private { } } elsif ($behaviour eq 'notice') { $meta->{variable} = 'false'; - $meta->{description} = $c->get_param("metadata[$i].description"); + my $desc = $c->get_param("metadata[$i].description"); + $meta->{description} = FixMyStreet::Template::sanitize($desc); $meta->{disable_form} = $c->get_param("metadata[$i].disable_form") ? 'true' : 'false'; } elsif ($behaviour eq 'hidden') { $meta->{automated} = 'hidden_field'; diff --git a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm index ea03b146f..3b7739966 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm @@ -286,6 +286,7 @@ sub update_contact : Private { # Special form disabling form if ($c->get_param('disable')) { my $msg = $c->get_param('disable_message'); + $msg = FixMyStreet::Template::sanitize($msg); $errors{category} = _('Please enter a message') unless $msg; my $meta = { code => '_fms_disable_', diff --git a/perllib/FixMyStreet/Template.pm b/perllib/FixMyStreet/Template.pm index 84faeb562..afab83e41 100644 --- a/perllib/FixMyStreet/Template.pm +++ b/perllib/FixMyStreet/Template.pm @@ -6,6 +6,7 @@ use warnings; use FixMyStreet; use mySociety::Locale; use Attribute::Handlers; +use HTML::Scrubber; use FixMyStreet::Template::SafeString; use FixMyStreet::Template::Context; use FixMyStreet::Template::Stash; @@ -135,4 +136,20 @@ sub html_paragraph : Filter('html_para') { return FixMyStreet::Template::SafeString->new($text); } +sub sanitize { + my $text = shift; + + my %allowed_tags = map { $_ => 1 } qw( p ul ol li br b i strong em ); + my $scrubber = HTML::Scrubber->new( + rules => [ + %allowed_tags, + a => { href => qr{^(http|/|tel)}i, style => 1, target => qr/^_blank$/, title => 1 }, + font => { color => 1 }, + span => { style => 1 }, + ] + ); + $text = $scrubber->scrub($text); + return $text; +} + 1; diff --git a/perllib/Open311/PopulateServiceList.pm b/perllib/Open311/PopulateServiceList.pm index 7c4337b1a..3e987b7dd 100644 --- a/perllib/Open311/PopulateServiceList.pm +++ b/perllib/Open311/PopulateServiceList.pm @@ -246,7 +246,13 @@ sub _add_meta_to_contact { # turn the data into something a bit more friendly to use @meta = # remove trailing colon as we add this when we display so we don't want 2 - map { $_->{description} =~ s/:\s*$// if $_->{description}; $_ } + map { + if ($_->{description}) { + $_->{description} =~ s/:\s*$//; + $_->{description} = FixMyStreet::Template::sanitize($_->{description}); + } + $_ + } # there is a display order and we only want to sort once sort { ($a->{order} || 0) <=> ($b->{order} || 0) } @meta; diff --git a/t/app/controller/admin/bodies.t b/t/app/controller/admin/bodies.t index 2d09ee85e..340351473 100644 --- a/t/app/controller/admin/bodies.t +++ b/t/app/controller/admin/bodies.t @@ -236,13 +236,13 @@ subtest 'disable form message editing' => sub { $mech->get_ok('/admin/body/' . $body->id . '/test%20category'); $mech->submit_form_ok( { with_fields => { disable => 1, - disable_message => 'Please ring us!', + disable_message => '<em>Please</em> <u>ring</u> us on <a href="tel:01234">01234</a>, click <a href="javascript:bad">bad</a>', note => 'Adding emergency message', } } ); $mech->content_contains('Values updated'); my $contact = $body->contacts->find({ category => 'test category' }); is_deeply $contact->get_extra_fields, [{ - description => 'Please ring us!', + description => '<em>Please</em> ring us on <a href="tel:01234">01234</a>, click <a>bad</a>', code => '_fms_disable_', protected => 'true', variable => 'false', diff --git a/t/app/controller/admin/reportextrafields.t b/t/app/controller/admin/reportextrafields.t index 388cc3c95..dede25207 100644 --- a/t/app/controller/admin/reportextrafields.t +++ b/t/app/controller/admin/reportextrafields.t @@ -68,7 +68,7 @@ FixMyStreet::override_config { "metadata[9999].code" => "string_test", "metadata[9999].required" => 1, "metadata[9999].behaviour" => "question", - "metadata[9999].description" => "this is a test description", + "metadata[9999].description" => "<div style='foo'>this is a test description</div>", "metadata[9999].datatype" => "string", "note" => "Added extra field", }}); diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t index 75c468666..40e124d03 100644 --- a/t/open311/populate-service-list.t +++ b/t/open311/populate-service-list.t @@ -430,7 +430,7 @@ for my $test ( required => 'true', datatype_description => 'Type of bin', order => 1, - description => 'Type of bin' + description => 'Type of <b>bin</b>' } ], meta_xml => '<?xml version="1.0" encoding="utf-8"?> @@ -444,7 +444,7 @@ for my $test ( <required>true</required> <datatype_description>Type of bin</datatype_description> <order>1</order> - <description>Type of bin</description> + <description><type>Type</type> of <b>bin</b></description> </attribute> </attributes> </service_definition> |