error on bad update ids passed to contact form

Return an error if an id for either a hidden update or one not
associated with the problem id is passed to the contact form.

3 files changed, 73 insertions, 9 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d2793b2a8..77dcbba2e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,8 @@
 * Unreleased
     - Front end improvements:
       - Extra help text on contact form #2149
+    - Bugfixes:
+      - Prevent contact form leaking information about updates #2149
 
 * v2.3.2 (31st May 2018)
     - Front end improvements:
diff --git a/perllib/FixMyStreet/App/Controller/Contact.pm b/perllib/FixMyStreet/App/Controller/Contact.pm
index b124ba1c0..997009b87 100644
--- a/perllib/FixMyStreet/App/Controller/Contact.pm
+++ b/perllib/FixMyStreet/App/Controller/Contact.pm
@@ -87,9 +87,17 @@ sub determine_contact_type : Private {
     } elsif ($id) {
         $c->forward( '/report/load_problem_or_display_error', [ $id ] );
         if ($update_id) {
-            my $update = $c->model('DB::Comment')->find(
-                { id => $update_id }
-            );
+            my $update = $c->model('DB::Comment')->search(
+                {
+                    id => $update_id,
+                    problem_id => $id,
+                    state => 'confirmed',
+                }
+            )->first;
+
+            unless ($update) {
+                $c->detach( '/page_error_404_not_found', [ _('Unknown update ID') ] );
+            }
 
             $c->stash->{update} = $update;
         }
diff --git a/t/app/controller/contact.t b/t/app/controller/contact.t
index c1039d15b..4f255f058 100644
--- a/t/app/controller/contact.t
+++ b/t/app/controller/contact.t
@@ -37,6 +37,17 @@ for my $test (
         detail    => 'More detail on the different problem',
         postcode  => 'EH99 1SP',
         confirmed => '2011-05-03 13:24:28.145168',
+        anonymous => 0,
+        hidden    => 1,
+        meta      => 'Reported anonymously at 13:24, Tue  3 May 2011',
+    },
+    {
+        name      => 'A User',
+        email     => 'problem_report_test@example.com',
+        title     => 'A different problem',
+        detail    => 'More detail on the different problem',
+        postcode  => 'EH99 1SP',
+        confirmed => '2011-05-03 13:24:28.145168',
         anonymous => 1,
         meta      => 'Reported anonymously at 13:24, Tue  3 May 2011',
         update    => {
@@ -45,6 +56,38 @@ for my $test (
             text  => 'This is an update',
         },
     },
+    {
+        name      => 'A User',
+        email     => 'problem_report_test@example.com',
+        title     => 'A different problem',
+        detail    => 'More detail on the different problem',
+        postcode  => 'EH99 1SP',
+        confirmed => '2011-05-03 13:24:28.145168',
+        anonymous => 1,
+        meta      => 'Reported anonymously at 13:24, Tue  3 May 2011',
+        update    => {
+            other_problem => 1,
+            name  => 'Different User',
+            email => 'commenter@example.com',
+            text  => 'This is an update',
+        },
+    },
+    {
+        name      => 'A User',
+        email     => 'problem_report_test@example.com',
+        title     => 'A different problem',
+        detail    => 'More detail on the different problem',
+        postcode  => 'EH99 1SP',
+        confirmed => '2011-05-03 13:24:28.145168',
+        anonymous => 1,
+        meta      => 'Reported anonymously at 13:24, Tue  3 May 2011',
+        update    => {
+            hidden => 1,
+            name  => 'Different User',
+            email => 'commenter@example.com',
+            text  => 'This is an update',
+        },
+    },
   )
 {
     subtest 'check reporting a problem displays correctly' => sub {
@@ -58,7 +101,7 @@ for my $test (
                 confirmed => $test->{confirmed},
                 name      => $test->{name},
                 anonymous => $test->{anonymous},
-                state     => 'confirmed',
+                state     => $test->{hidden} ? 'hidden' : 'confirmed',
                 user      => $user,
                 latitude  => 0,
                 longitude => 0,
@@ -76,9 +119,9 @@ for my $test (
 
             $update = FixMyStreet::App->model('DB::Comment')->create(
                 {
-                    problem_id => $problem->id,
+                    problem_id => $update_info->{other_problem} ? $problem_main->id : $problem->id,
                     user        => $update_user,
-                    state       => 'confirmed',
+                    state       => $update_info->{hidden} ? 'hidden' : 'confirmed',
                     text        => $update_info->{text},
                     confirmed   => \'current_timestamp',
                     mark_fixed => 'f',
@@ -90,9 +133,20 @@ for my $test (
         ok $problem, 'succesfully create a problem';
 
         if ( $update ) {
-            $mech->get_ok( '/contact?id=' . $problem->id . '&update_id=' . $update->id );
-            $mech->content_contains('reporting the following update');
-            $mech->content_contains( $test->{update}->{text} );
+            if ( $test->{update}->{hidden} ) {
+                $mech->get( '/contact?id=' . $problem->id . '&update_id=' . $update->id );
+                is $mech->res->code, 404, 'cannot report a hidden update';
+            } elsif ( $test->{update}->{other_problem} ) {
+                $mech->get( '/contact?id=' . $problem->id . '&update_id=' . $update->id );
+                is $mech->res->code, 404, 'cannot view an update for another problem';
+            } else {
+                $mech->get_ok( '/contact?id=' . $problem->id . '&update_id=' . $update->id );
+                $mech->content_contains('reporting the following update');
+                $mech->content_contains( $test->{update}->{text} );
+            }
+        } elsif ( $test->{hidden} ) {
+            $mech->get( '/contact?id=' . $problem->id );
+            is $mech->res->code, 410, 'cannot report a hidden problem';
         } else {
             $mech->get_ok( '/contact?id=' . $problem->id );
             $mech->content_contains('reporting the following problem');