Bug 1064140 (CVE-2014-1571)

[SECURITY] Private comments can be shown to flagmail recipients who aren't in the insider group

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
Email Notifications
P1
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: glob, Assigned: Simon Green)

Tracking

({sec-high})

2.17.1
Bugzilla 4.0
sec-high
Dependency tree / graph
Bug Flags:
approval +
approval4.4 +
blocking4.4.6 +
approval4.2 +
blocking4.2.11 +
approval4.0 +
blocking4.0.15 +

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
template/en/default/email/flagmail.txt.tmpl has:

> [% IF Bugzilla.cgi.param("comment").defined && Bugzilla.cgi.param("comment").length > 0 %]
> ------- Additional Comments from [% user.identity %]
> [%+ Bugzilla.cgi.param("comment") FILTER strip_control_chars %]

it's highly likely this will not work for API driven updates.
(Assignee)

Comment 1

3 years ago
Setting the security flag on this because of the follow situation:

User A is in the insider group. User B can see the bug, but is not in the insider group.

UserA make a private comment and sets the needinfo? flag, with User B as the requestee. User B receives the flag mail but it includes the private comment despite them not been able to see the private comment otherwise. The bug mail is correct (shows all changes but not the private comment)
Group: bugzilla-security
Target Milestone: --- → Bugzilla 4.0
(Assignee)

Updated

3 years ago
Assignee: email-notifications → sgreen
Severity: normal → critical
Status: NEW → ASSIGNED
Depends on: 179582
Keywords: sec-high
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
(Assignee)

Updated

3 years ago
Summary: flagmail email template shouldn't read data from $cgi → Private comments can be shown to flagmail recipients who aren't in the insider group
(Assignee)

Comment 2

3 years ago
Created attachment 8485611 [details] [diff] [review]
v1 patch

There were two issues that needed to worked around:

$bug->update added the flags before the comments. I solved this by moving the flag change to further down the sub routine.

The second issues was the $bug->{comments} was cached from before the change. This was solved by adding a nocache option.

Ideally, we should move flag mail generation out of Bugzilla::Flag and into send_changes, but that is for a different bug. Once this review is approved, I'll supply patches for 4.0, 4.2 and 4.4 if needed.
Attachment #8485611 - Flags: review?(glob)
(Reporter)

Updated

3 years ago
Blocks: 1046280
(Reporter)

Comment 3

3 years ago
Comment on attachment 8485611 [details] [diff] [review]
v1 patch

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

aside from the nocache change, this patch looks good.

::: Bugzilla/Flag.pm
@@ +1124,5 @@
>      }
>  
> +    # Get comments on the bug
> +    my $all_comments = $bug->comments({ after => $bug->lastdiffed, nocache => 1 });
> +    @$all_comments   = grep { $_->type || $_->body =~ /\S/ } @$all_comments;

to avoid re-fetching the comments, it should be easier to delete $bug->{comments} when a comment is added.
Attachment #8485611 - Flags: review?(glob) → review-

Comment 4

3 years ago
(In reply to Byron Jones ‹:glob› from comment #0)
> template/en/default/email/flagmail.txt.tmpl has:
> it's highly likely this will not work for API driven updates.

FYI, this has already been filed as bug 508541 (and a dupe already exists: bug 1054181).
(Assignee)

Updated

3 years ago
Blocks: 508541
(Assignee)

Comment 5

3 years ago
Created attachment 8486131 [details] [diff] [review]
v2 patch
Attachment #8485611 - Attachment is obsolete: true
Attachment #8486131 - Flags: review?(glob)
(Reporter)

Comment 6

3 years ago
Comment on attachment 8486131 [details] [diff] [review]
v2 patch

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

r=glob
Attachment #8486131 - Flags: review?(glob) → review+
(Reporter)

Updated

3 years ago
Flags: approval4.4+
Flags: approval4.2+
Flags: approval4.0+
Flags: approval+

Updated

3 years ago
Blocks: 1072497
Assigned CVE-2014-1571 to this issue.
Alias: CVE-2014-1571

Comment 8

3 years ago
Created attachment 8495665 [details] [diff] [review]
Bugzilla 4.0 patch
Attachment #8495665 - Flags: review?(glob)

Comment 9

3 years ago
Created attachment 8495679 [details] [diff] [review]
Bugzilla 4.2 patch
Attachment #8495679 - Flags: review?(glob)

Comment 10

3 years ago
Created attachment 8495684 [details] [diff] [review]
Bugzilla 4.4 patch.

I've attached patches for Bugzilla 4.0, 4.2 and 4.4.

There's no functional changes, code has moved around and the trunk patch didn't apply cleanly to older branches.
Attachment #8495684 - Flags: review?(glob)
(Reporter)

Updated

3 years ago
Attachment #8495679 - Attachment is patch: true
(Reporter)

Comment 11

3 years ago
Comment on attachment 8495665 [details] [diff] [review]
Bugzilla 4.0 patch

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

r=glob
Attachment #8495665 - Flags: review?(glob) → review+
(Reporter)

Comment 12

3 years ago
Comment on attachment 8495679 [details] [diff] [review]
Bugzilla 4.2 patch

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

r=glob
Attachment #8495679 - Flags: review?(glob) → review+
(Reporter)

Comment 13

3 years ago
Comment on attachment 8495684 [details] [diff] [review]
Bugzilla 4.4 patch.

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

r=glob
Attachment #8495684 - Flags: review?(glob) → review+

Updated

3 years ago
Summary: Private comments can be shown to flagmail recipients who aren't in the insider group → [SECURITY] Private comments can be shown to flagmail recipients who aren't in the insider group
Version: unspecified → 2.17.1

Updated

3 years ago
Flags: blocking4.4.6?
Flags: blocking4.2.11?
Flags: blocking4.0.15?

Updated

3 years ago
Flags: blocking4.4.6?
Flags: blocking4.4.6+
Flags: blocking4.2.11?
Flags: blocking4.2.11+
Flags: blocking4.0.15?
Flags: blocking4.0.15+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   c8f8d1e..19576a8  4.0 -> 4.0

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   6d6b390..976dc12  4.2 -> 4.2

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   6364067..a92eee1  4.4 -> 4.4

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   f33b119..fa954ab  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Updated

3 years ago
Group: bugzilla-security
(Reporter)

Updated

3 years ago
Blocks: 1082887
You need to log in before you can comment on or make changes to this bug.