Bug 1832783 - Reject oversized CGI requests

Bugzilla/App/Controller/API.pm

@@ -17,8 +17,10 @@ use Try::Tiny;
 
 use Bugzilla::Constants;
 use Bugzilla::Logging;
+use Bugzilla::WebService::Util qw(set_rest_cors_headers);
 
 use constant SUPPORTED_VERSIONS => qw(V1);
+use constant REQUEST_TOO_LARGE_ERROR => 'request_too_large';
 
 sub setup_routes {
   my ($class, $r) = @_;
@@ -30,38 +32,17 @@ sub setup_routes {
   $r->namespaces($namespaces);
 
   # Backwards compat with /api/user/profile which Phabricator requires
-  $r->under(
-    '/api' => sub {
-      my ($c) = @_;
-      _insert_rest_headers($c);
-      Bugzilla->usage_mode(USAGE_MODE_REST);
-    }
-  )->get('/user/profile')->to('V1::User#user_profile');
+  $r->under('/api' => \&_prepare_rest_request)
+    ->get('/user/profile')->to('V1::User#user_profile');
 
   # Other backwards compat routes
-  $r->under(
-    '/latest' => sub {
-      my ($c) = @_;
-      _insert_rest_headers($c);
-      Bugzilla->usage_mode(USAGE_MODE_REST);
-    }
-  )->get('/configuration')->to('V1::Configuration#configuration');
-  $r->under(
-    '/bzapi' => sub {
-      my ($c) = @_;
-      _insert_rest_headers($c);
-      Bugzilla->usage_mode(USAGE_MODE_REST);
-    }
-  )->get('/configuration')->to('V1::Configuration#configuration');
+  $r->under('/latest' => \&_prepare_rest_request)
+    ->get('/configuration')->to('V1::Configuration#configuration');
+  $r->under('/bzapi' => \&_prepare_rest_request)
+    ->get('/configuration')->to('V1::Configuration#configuration');
 
   # Set the usage mode for all routes under /rest
-  my $rest_routes = $r->under(
-    '/rest' => sub {
-      my ($c) = @_;
-      _insert_rest_headers($c);
-      Bugzilla->usage_mode(USAGE_MODE_REST);
-    }
-  );
+  my $rest_routes = $r->under('/rest' => \&_prepare_rest_request);
 
   # Standard API support
   foreach my $version (SUPPORTED_VERSIONS) {
@@ -89,6 +70,21 @@ sub setup_routes {
   }
 }
 
+sub _prepare_rest_request {
+  my ($c) = @_;
+  _insert_rest_headers($c);
+
+  if ($c->req->is_limit_exceeded) {
+    my $reason = $c->req->error->{message};
+    WARN("Rejected oversized request for native REST API: $reason");
+    Bugzilla->usage_mode(USAGE_MODE_MOJO_REST);
+    return $c->user_error(REQUEST_TOO_LARGE_ERROR);
+  }
+
+  Bugzilla->usage_mode(USAGE_MODE_REST);
+  return 1;
+}
+
 sub _load_api_module {
   my ($routes, $module) = @_;
   try {
@@ -105,15 +101,9 @@ sub _load_api_module {
 
 sub _insert_rest_headers {
   my ($c) = @_;
-
-  # Access Control
   my @allowed_headers
     = qw(accept authorization content-type origin user-agent x-bugzilla-api-key x-requested-with);
-  $c->res->headers->header('Access-Control-Allow-Origin' => '*');
-  $c->res->headers->header(
-    'Access-Control-Allow-Headers' => join ', ',
-    @allowed_headers
-  );
+  set_rest_cors_headers($c->res->headers, \@allowed_headers);
 }
 
 1;

Bugzilla/App/Controller/CGI.pm

@@ -17,9 +17,17 @@ use Socket qw(AF_INET inet_aton);
 use Mojo::File qw(path);
 use English qw(-no_match_vars);
 use Bugzilla::App::Stdout;
-use Bugzilla::Constants qw(bz_locations USAGE_MODE_BROWSER);
+use Bugzilla::Constants qw(
+  bz_locations
+  USAGE_MODE_BROWSER
+  USAGE_MODE_MOJO
+  USAGE_MODE_MOJO_REST
+);
+use Bugzilla::Logging;
+use Bugzilla::WebService::Util qw(set_rest_cors_headers);
 
 my %SEEN;
+use constant REQUEST_TOO_LARGE_ERROR => 'request_too_large';
 
 sub setup_routes {
   my ($class, $r) = @_;
@@ -60,6 +68,13 @@ sub load_one {
   my $inner = quote_sub $inner_name, $content, {}, \%options;
   my $wrapper = sub {
     my ($c) = @_;
+
+    if ($c->req->is_limit_exceeded) {
+      my $reason = $c->req->error->{message};
+      WARN("Rejected oversized request for $file: $reason");
+      return _render_request_too_large($c, $file);
+    }
+
     Bugzilla->request_cache->{mojo_controller} = $c;
     my $stdin = $c->_STDIN;
     local %ENV                         = $c->_ENV($file);
@@ -94,6 +109,23 @@ sub load_one {
   return 1;
 }
 
+sub _render_request_too_large {
+  my ($c, $file) = @_;
+
+  if (path($file)->basename eq 'rest.cgi') {
+    set_rest_cors_headers($c->res->headers);
+    Bugzilla->usage_mode(USAGE_MODE_MOJO_REST);
+    return $c->user_error(REQUEST_TOO_LARGE_ERROR);
+  }
+
+  Bugzilla->usage_mode(USAGE_MODE_MOJO);
+  return $c->user_error(
+    REQUEST_TOO_LARGE_ERROR,
+    {},
+    {status => 413, skip_exception_page => 1}
+  );
+}
+
 sub _ENV {
   my ($c, $script_name) = @_;
   my $tx      = $c->tx;

Bugzilla/App/Plugin/Error.pm

@@ -28,7 +28,8 @@ sub register {
 }
 
 sub _render_error {
-  my ($type, $c, $error, $vars) = @_;
+  my ($type, $c, $error, $vars, $options) = @_;
+  $options ||= {};
 
   # If values are defined in the stash, use those instead
   my $stash = $c->stash;
@@ -49,7 +50,7 @@ sub _render_error {
   if (Bugzilla->usage_mode == USAGE_MODE_MOJO) {
     $logfunc->("webpage error: $error");
 
-    if ($c->app->mode eq 'development') {
+    if ($c->app->mode eq 'development' && !$options->{skip_exception_page}) {
       use Bugzilla::Logging;
       my $class = $type ? 'Bugzilla::Error::' . ucfirst($type) : 'Mojo::Exception';
       my $e     = $class->new($error)->trace(2);
@@ -63,8 +64,8 @@ sub _render_error {
         template => "global/$type-error",
         format   => 'html',
         error    => $error,
-        status   => 200,
-        %{$vars}
+        %{$vars},
+        status => $options->{status} // $vars->{status} // 200
       );
       return 0;
     }

Bugzilla/WebService/Constants.pm

@@ -70,6 +70,7 @@ use constant WS_ERROR_CODE => {
   number_too_small      => 55,
   illegal_date          => 56,
   illegal_date_pronoun  => 57,
+  request_too_large     => 58,
 
   # Bug errors usually occupy the 100-200 range.
   improper_bug_id_field_value => 100,
@@ -255,6 +256,7 @@ use constant STATUS_BAD_REQUEST      => 400;
 use constant STATUS_NOT_AUTHORIZED   => 401;
 use constant STATUS_NOT_FOUND        => 404;
 use constant STATUS_GONE             => 410;
+use constant STATUS_REQUEST_TOO_LARGE => 413;
 
 # The integer value is the error code above returned by
 # the related webvservice call. We choose the appropriate
@@ -263,6 +265,7 @@ use constant STATUS_GONE             => 410;
 sub REST_STATUS_CODE_MAP {
   my $status_code_map = {
     51       => STATUS_NOT_FOUND,
+    58       => STATUS_REQUEST_TOO_LARGE,
     101      => STATUS_NOT_FOUND,
     102      => STATUS_NOT_AUTHORIZED,
     106      => STATUS_NOT_AUTHORIZED,

Bugzilla/WebService/Server/REST.pm

@@ -19,7 +19,7 @@ use Bugzilla::Error;
 use Bugzilla::Hook;
 use Bugzilla::Util qw(html_quote disable_utf8 enable_utf8);
 use Bugzilla::WebService::Constants;
-use Bugzilla::WebService::Util qw(taint_data fix_credentials);
+use Bugzilla::WebService::Util qw(fix_credentials set_rest_cors_headers taint_data);
 
 # Load resource modules
 use Bugzilla::WebService::Server::REST::Resources::Bug;
@@ -141,18 +141,7 @@ sub response {
   Bugzilla::Hook::process('webservice_rest_response',
     {rpc => $self, result => \$result, response => $response});
 
-  # Access Control
-  my @allowed_headers
-    = qw(accept content-type origin user-agent x-requested-with);
-  foreach my $header (keys %{API_AUTH_HEADERS()}) {
-
-    # We want to lowercase and replace _ with -
-    my $translated_header = $header;
-    $translated_header =~ tr/A-Z_/a-z\-/;
-    push(@allowed_headers, $translated_header);
-  }
-  $response->header("Access-Control-Allow-Origin", "*");
-  $response->header("Access-Control-Allow-Headers", join(', ', @allowed_headers));
+  set_rest_cors_headers($response);
 
   # ETag support
   my $etag = $self->bz_etag;

Bugzilla/WebService/Util.pm

@@ -37,8 +37,26 @@ our @EXPORT_OK = qw(
   translate
   params_to_objects
   fix_credentials
+  set_rest_cors_headers
 );
 
+sub set_rest_cors_headers {
+  my ($headers, $allowed_headers) = @_;
+  if (!$allowed_headers) {
+    my @default_headers
+      = qw(accept authorization content-type origin user-agent x-requested-with);
+    foreach my $header (sort keys %{API_AUTH_HEADERS()}) {
+      $header =~ tr/A-Z_/a-z\-/;
+      push @default_headers, $header;
+    }
+    $allowed_headers = \@default_headers;
+  }
+
+  $headers->header('Access-Control-Allow-Origin' => '*');
+  $headers->header(
+    'Access-Control-Allow-Headers' => join(', ', @$allowed_headers));
+}
+
 sub extract_flags {
   my ($flags, $bug, $attachment) = @_;
   my (@new_flags, @old_flags);