Bug 657158 (CVE-2011-2381)

[SECURITY] Request email headers for attachment containing newline are corrupt

VERIFIED FIXED in Bugzilla 3.4

Status

()

Bugzilla
Email Notifications
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: reed)

Tracking

2.17.1
Bugzilla 3.4
Bug Flags:
approval +
blocking4.2 +
approval4.0 +
blocking4.0.2 +
approval3.6 +
blocking3.6.6 +
approval3.4 +
blocking3.4.12 +

Details

(Whiteboard: [infrasec:input][ws:low])

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
I received a request for an attachment with the following description:
>Read the ignored data
>[Checked in: See comment 18]
This caused the notification email to be corrupted.
I don't remember seeing this corruption before so I think it's a regression.
(Reporter)

Comment 1

6 years ago
Created attachment 532459 [details]
Example of a corrupt message

As you can see, the [Checked in: See comment 18] has been output on a separate line and things have gone downhill from there.

Comment 2

6 years ago
Looks specific to bmo. I cannot reproduce upstream. In both 4.0 and 4.1 is the newline removed, and the email is correctly formatted.
Assignee: email-notifications → nobody
Component: Email Notifications → General
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa → general
Version: 4.0 → Current
(Assignee)

Comment 3

6 years ago
Nope, I can confirm this on landfill. Definitely an upstream issue. Also marking this security-sensitive, as you could modify contents of e-mail headers this way.

I don't think it's a regression, but I haven't tested enough to confirm.
Assignee: nobody → email-notifications
No longer blocks: 652325
Group: bugzilla-security
Component: General → Email Notifications
Product: bugzilla.mozilla.org → Bugzilla
QA Contact: general → default-qa
Target Milestone: --- → Bugzilla 4.0
Version: Current → 4.0
(Assignee)

Updated

6 years ago
Flags: blocking4.2?
Flags: blocking4.0.2?
Summary: Request email headers for attachment containing newline are corrupt → [SECURITY] Request email headers for attachment containing newline are corrupt
Version: 4.0 → 4.0.1
(Assignee)

Updated

6 years ago
Keywords: regression
Whiteboard: [infrasec:input][ws:low]
(Assignee)

Comment 4

6 years ago
This doesn't affect things like the short_desc, as clean_text() is run as part of the validator (disallowing any control characters, including newlines). We don't do the same for attachment descriptions, however.
(Assignee)

Comment 5

6 years ago
Created attachment 532492 [details] [diff] [review]
patch for 4.x - v1

Something like this, maybe? I haven't tested it yet.
Assignee: email-notifications → reed
Status: NEW → ASSIGNED
Attachment #532492 - Flags: review?(mkanat)

Comment 6

6 years ago
(In reply to comment #5)
> Something like this, maybe? I haven't tested it yet.

I would like that you test your patches first. I saw too many regressions recently because such patches haven't been tested.

Also, I need exact STR as I couldn't reproduce it. This will help to know which branches are affected.
(Assignee)

Comment 7

6 years ago
(In reply to comment #6)
> (In reply to comment #5)
> > Something like this, maybe? I haven't tested it yet.
> 
> I would like that you test your patches first. I saw too many regressions
> recently because such patches haven't been tested.

My time is extremely limited. I specifically called out the fact that my patch is untested. Considering this is a *security* bug, I felt that putting a potential patch up for review first is a higher priority than waiting to test it. You're welcome to test it yourself if you have more time than I do or have concerns about my work.

(In reply to comment #6)
> Also, I need exact STR as I couldn't reproduce it. This will help to know
> which branches are affected.

https://landfill.bugzilla.org/bugzilla-4.0-branch/show_bug.cgi?id=333 has my testing. I created a new attachment, went to the attachment details, modified the description to include a new line, and requested review from myself. The subsequent review mail I received included the same type of problems that attachment 532459 [details] shows.

Comment 8

6 years ago
(In reply to comment #7)
> My time is extremely limited.

We all have limited time.


> Considering this is a *security* bug,

It's not a security bug. You cannot do any harm besides moving some headers into the body of the email. And the bug exists at least since Bugzilla 3.2, where I can reproduce the issue, and probably existed since 2.18, when flags have been introduced.


> a potential patch up for review first is a higher priority than waiting to
> test it.

I disagree. Asking someone for review when the patch may not (fully) work is more time consuming as reviewers have even less time for reviews.


> or have concerns about my work.

I have concerns about the work of everbody who attaches untested patches. It's not just about you. It's not my job to catch regressions or to realize that a problem is not fully fixed. This makes us waste more time later.


> I created a new attachment, went to the attachment details,
> modified the description to include a new line, and requested review from
> myself. The subsequent review mail I received included the same type of
> problems that attachment 532459 [details] shows.

That's interesting, the newline doesn't appear in all cases. If I use the exact same description as you, I can reproduce. If I use "patch, v1\n[foo bar]", I cannot. No idea why. Maybe it depends on which position the newline is in the string.

As there is no reason to forbird newlines in attachment descriptions for now, filtering them in the email template is the right thing to do.
Group: bugzilla-security
Flags: blocking4.2?
Flags: blocking4.2+
Flags: blocking4.0.2?
Flags: blocking4.0.2+
Version: 4.0.1 → 3.2.10

Updated

6 years ago
Summary: [SECURITY] Request email headers for attachment containing newline are corrupt → Request email headers for attachment containing newline are corrupt
(Assignee)

Comment 9

6 years ago
(In reply to comment #8)
> > Considering this is a *security* bug,
> 
> It's not a security bug. You cannot do any harm besides moving some headers
> into the body of the email. And the bug exists at least since Bugzilla 3.2,
> where I can reproduce the issue, and probably existed since 2.18, when flags
> have been introduced.

Wrong, wrong, 100% wrong. This is most definitely a security bug. Injection of headers into e-mail messages is a very bad thing. Spamming is the first thing that comes to mind, but I can also think of other nefarious things.
Group: bugzilla-security
Summary: Request email headers for attachment containing newline are corrupt → [SECURITY] Request email headers for attachment containing newline are corrupt

Comment 10

6 years ago
  There's no good reason to allow newlines into attachment descriptions, we can clean them out on input, that's fine with me.

Comment 11

6 years ago
Comment on attachment 532492 [details] [diff] [review]
patch for 4.x - v1

  For trunk, I'd actually rather do it in the validator in Bugzilla::Attachment, although what you're doing here is probably good for the branches.

Comment 12

6 years ago
(In reply to comment #10)
>   There's no good reason to allow newlines into attachment descriptions

In that case, all existing attachment descriptions should be fixed, and the textarea should be replaced by a text field.
Moving to the Bugzilla product since this was acknowledged as an upstream bug.
Assignee: reed → administration
Component: Email Notifications → Administration
Version: 3.2.10 → 2.10
Interesting that it never prompted me to verify the version and component when I moved products...
Version: 2.10 → 3.2

Comment 15

6 years ago
drug is bad. It's already upstream.
(Assignee)

Updated

6 years ago
Assignee: administration → reed
Component: Administration → Email Notifications
Version: 3.2 → 3.2.10

Comment 16

6 years ago
glob and I managed to inject headers in emails, which is worse than having some headers in the body of the email. So now I agree to call it a security bug. :)
Flags: blocking3.6.6+
Flags: blocking3.4.12+
Target Milestone: Bugzilla 4.0 → Bugzilla 3.4

Comment 17

6 years ago
Comment on attachment 532492 [details] [diff] [review]
patch for 4.x - v1

Works fine on trunk. Also applies cleanly to 4.0. But the patch doesn't apply cleanly to 3.x. r=LpSolit for 4.x.

I think that removing newlines from attachment descriptions should be done as a separate bug.
Attachment #532492 - Attachment description: patch - v1 (untested) → patch for 4.x - v1
Attachment #532492 - Attachment filename: 657158 → bug657158.diff
Attachment #532492 - Flags: review?(mkanat) → review+

Comment 18

6 years ago
Backports for 3.6 and 3.4 needed.
Flags: approval?
Flags: approval4.0?

Comment 19

6 years ago
(In reply to comment #17)
> I think that removing newlines from attachment descriptions should be done
> as a separate bug.

  That sounds great to me; I agree.

Comment 20

6 years ago
reed: have time for the backports?

Updated

6 years ago
Blocks: 660528

Comment 21

6 years ago
Created attachment 544248 [details] [diff] [review]
patch for 3.x, v1

reed said he was too busy, so here is the backport for 3.4 and 3.6. Tested on both branches, works fine.
Attachment #544248 - Flags: review?(glob)
Comment on attachment 544248 [details] [diff] [review]
patch for 3.x, v1

r=glob
Attachment #544248 - Flags: review?(glob) → review+
Flags: approval3.6?
Flags: approval3.4?
Use CVE-2011-2381 for this bug.
Alias: CVE-2011-2381

Comment 24

6 years ago
2.17.1 was the first release to have this problem, see bug 98801. At that time, this template was split into two pieces: request/created-email.txt.tmpl and request/fulfilled-email.txt.tmpl. Both templates had this problem already.
Version: 3.2.10 → 2.17.1

Updated

6 years ago
Flags: approval?
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval3.4?
Flags: approval3.4+
Flags: approval+
(Assignee)

Comment 25

6 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Template.pm
modified template/en/default/request/email.txt.tmpl
Committed revision 7887.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Template.pm
modified template/en/default/request/email.txt.tmpl
Committed revision 7633.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/Template.pm
modified template/en/default/request/email.txt.tmpl
Committed revision 7250.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.4/
modified Bugzilla/Template.pm
modified template/en/default/request/email.txt.tmpl
Committed revision 6804.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 26

6 years ago
Security advisory sent, unlocking this bug.
Group: bugzilla-security
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.