Set $CGI::POST_MAX to mitigate DoS attacks

NEW
Unassigned

Status

()

Bugzilla
Bugzilla-General
--
enhancement
7 years ago
5 years ago

People

(Reporter: Frédéric Buclin, Unassigned)

Tracking

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
$CGI::POST_MAX lets us limit the size of POST data passed to CGI scripts. If we exceed this limit, CGI.pm throws a "HTTP/1.1 413 Request entity too large" error. We could use it to prevent Bugzilla from being DoS'ed.

The biggest thing we can have in Bugzilla is attachments. If we include the comment which can be passed at the same time, I would say

  $CGI::POST_MAX = 2* max(Param('maxattachmentsize'), Param('maxlocalattachment'))

would be a reasonable limit.

Comment 1

7 years ago
Don't we already set POST_MAX? (In order to be able to upload attachments at all.)
(Reporter)

Comment 2

7 years ago
(In reply to comment #1)
> Don't we already set POST_MAX? (In order to be able to upload attachments at
> all.)

We don't, no. Maybe you have MySQL's "max_allowed_packet" variable in mind?

Comment 3

7 years ago
(In reply to comment #2)
> We don't, no. Maybe you have MySQL's "max_allowed_packet" variable in mind?

  Huh. I think maybe we used to set it, back in the old days of like 2.18 or something. Maybe I realized it was unnecessary and removed it.

  I would be fine with 2 x max_attachment, that sounds like a good idea.
(Reporter)

Comment 4

7 years ago
Created attachment 575924 [details] [diff] [review]
patch, v1
Assignee: general → LpSolit
Status: NEW → ASSIGNED
Attachment #575924 - Flags: review?(mkanat)
(Reporter)

Updated

7 years ago
Target Milestone: --- → Bugzilla 5.0

Comment 5

7 years ago
Comment on attachment 575924 [details] [diff] [review]
patch, v1

Review of attachment 575924 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/CGI.pm
@@ +78,5 @@
>  
> +    # Limit the size of the POST data.
> +    $CGI::POST_MAX = 2 * max(1024 * Bugzilla->params->{maxattachmentsize},
> +                             1024 * 1024 * Bugzilla->params->{maxlocalattachment});
> +

Hmmm, there's no way to set this on $self instead, is there? This will affect other mod_perl apps on the same server, otherwise. Sometimes CGI.pm globals actually initialize some hidden variable in $self, so perhaps we could use that?
(Reporter)

Comment 6

6 years ago
Comment on attachment 575924 [details] [diff] [review]
patch, v1

Pushing this patch out of our radar for now, but we may revive it in the future if we have evidence of such problems.
Attachment #575924 - Flags: review?(mkanat)
(Reporter)

Updated

6 years ago
Assignee: LpSolit → general
Status: ASSIGNED → NEW
Target Milestone: Bugzilla 4.4 → ---
You need to log in before you can comment on or make changes to this bug.