481506 - The latest email address filtering changes causes a DoS problem when comments have certain text in them

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[David Lawrence [:dkl]]

Attachment: Test script that illustrates issue with certain text and Email::Address

The latest email address filtering changes in cvs causes a DoS problem when comments have certain text in them.

Since bug comments are now filtered using Bugzilla::Util::email_filter,
Email::Address is not able to handle some blocks of text properly and consumes
massive amounts of CPU and also denies access to the bug itself for others.

I am attaching a simple script that has the text from out site that causes the
issue.

Comment 1

[Frédéric Buclin]

I can reproduce the problem. Hard blocker!

Comment 2

[Frédéric Buclin]

The code "freezes" at Email::Address->parse().

Comment 3

[Frédéric Buclin]

rjbs: could you investigate? There is a serious problem with ->parse().

Comment 4

[Frédéric Buclin]

Huh? I added "print 1" right after |return unless $line;| in Email/Address.pm, and this line is never reached?!

Comment 5

[Frédéric Buclin]

From what I can see, the problem comes from:

  my (@mailboxes) = ($line =~ /$mailbox/go);

in parse().

Comment 6

[Frédéric Buclin]

Attachment: Hack for Email::Address->parse

$mailbox    = qr/(?:$name_addr|$addr_spec)$comment*/;

is the line which makes everything slow. If I split it as:

$mailbox1   = qr/$name_addr$comment*/;
$mailbox2   = qr/$addr_spec$comment*/;

it's fast again. But I don't know if this hack has the exact same effect as the original code or not (e.g. could it find the same mailbox twice?).

Comment 7

[Frédéric Buclin]

Attachment: Workaround, v1

Workaround: don't filter email addresses in comments. As said in bug 219021 comment 54, filtering email addresses in bug comments is only a bonus.

Comment 8

[:glob ✱]

Comment on attachment 365557 [details] [diff] [review]
Workaround, v1

r=glob

Comment 9

[Frédéric Buclin]

mkanat: do you agree to take this fix for now? We can revert it for Bugzilla 3.6 if the bug in Email::Address->parse is fixed meanwhile.

Comment 10

[Max Kanat-Alexander]

Yeah, I agree for now. :-)

Comment 11

[Frédéric Buclin]

Checking in template/en/default/bug/comments.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v  <--  comments.html.tmpl
new revision: 1.41; previous revision: 1.40
done

Comment 12

[David Lawrence [:dkl]]

This change needs to be in template/en/default/bug/show.xml.tmpl as well. That is where we first noticed it.

Dave

Comment 13

[Max Kanat-Alexander]

Okay, we should also fix anywhere that comments are shown, as dkl points out.

Comment 14

[Frédéric Buclin]

Attachment: Workaround part 2, v1

I checked, and that's the single other place where comments are filtered.

Comment 15

[Max Kanat-Alexander]

Comment on attachment 365580 [details] [diff] [review]
Workaround part 2, v1

Okay, sounds fine to me.

Comment 16

[Frédéric Buclin]

Checking in template/en/default/bug/show.xml.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.xml.tmpl,v  <--  show.xml.tmpl
new revision: 1.28; previous revision: 1.27
done