The default bug view has changed. See this FAQ.

[SECURITY] email_in.pl lets you set the changer as @reporter instead of only checking the "From" header

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Incoming Email
--
blocker
RESOLVED FIXED
9 years ago
9 years ago

People

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

Tracking

2.23.4
Bugzilla 3.0
Bug Flags:
approval +
blocking3.2 +
approval3.0 +
blocking3.0.4 +

Details

(Whiteboard: [ready for 3.x])

Attachments

(1 attachment, 1 obsolete attachment)

1.07 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
email_in.pl lets you do changes even without altering the email headers. This means that even if you use PGP to sign your emails, or use tokens, it's still trivial to alter bugs as if you were someone else. One useful case would be the ability to CC yourself to bugs you want to see. All you have to do is to add the following line to your email:

@reporter = someone.else@user.account

Where the email address is a valid user account with privs to access security bugs. This trick works with process_bug.cgi, i.e. with existing bugs, which is why I restricted this bug to the security group.

I know email_in.pl currently has no way to check the real identity of the sender, but for installations which added some checks to it and turned on incoming emails, they are still fully vulnerable (e.g. Mandriva Bugzilla).

Now technically, the reason this works is because we have:

    my $username = $mail_fields->{'reporter'};
    my $user = Bugzilla::User->new({ name => $username })
      || ThrowUserError('invalid_username', { name => $username });

    Bugzilla->set_user($user);


But $mail_fields->{'reporter'} comes from:

    my ($reporter) = Email::Address->parse($input_email->header('From'));
    $fields{'reporter'} = $reporter->address;

The problem is that later in the code, we have:

    if ($line =~ /^@(\S+)\s*=\s*(.*)\s*/) {
        $current_field = lc($1);
        $fields{$current_field} = $2;
    }

So if you have the @reporter line, it will override the one taken from the email header, and so all authenticated emails are vulnerable.

I just tried this trick to CC myself to a bug I'm not allowed to see on a test installation, and this works great. I could do the same on Mandriva!
Flags: blocking3.2+
Flags: blocking3.0.4+
(Assignee)

Comment 1

9 years ago
Created attachment 305215 [details] [diff] [review]
patch, v1

Disallow to pass the 'reporter' field. This one is now only set based on the "From": field.
Assignee: incoming.email → LpSolit
Status: NEW → ASSIGNED
Attachment #305215 - Flags: review?(mkanat)
(Assignee)

Comment 2

9 years ago
Created attachment 305220 [details] [diff] [review]
patch, v2

Even better fix: prevent the attacker to split the reporter field on several lines. To do that, I reset the $current_field variable.
Attachment #305215 - Attachment is obsolete: true
Attachment #305220 - Flags: review?(mkanat)
Attachment #305215 - Flags: review?(mkanat)
(Assignee)

Comment 3

9 years ago
CC'ing the Mandriva Bugzilla maintainer as he should be aware of the issue.

Comment 4

9 years ago
Thanks for the heads up on this.

I just applied the v2 patch and verified it works:

You are not authorized to access bug #38032.
From vdanen@gmail.com  Sat Feb 23 19:30:14 2008
 Subject: [Bug 38032]
  Folder: /var/www/html/bugzilla/bugzilla/email_in.pl			   2518

From vdanen@mandriva.com  Sat Feb 23 19:32:57 2008
 Subject: [Bug 38032]
  Folder: /var/www/html/bugzilla/bugzilla/email_in.pl			   2509


That's the output from procmail with the patch applied; previously I was able to send a comment with my gmail.com account which has no privs to the bug in question.

So I can verify that the fix works.

Thanks again for the heads up.

Comment 5

9 years ago
Comment on attachment 305220 [details] [diff] [review]
patch, v2

Cool, that looks good to me.
Attachment #305220 - Flags: review?(mkanat) → review+

Comment 6

9 years ago
Now, just to be clear about all this, you could already add yourself to any bug you can't see, by changing the "From" address. The email_in.pl docs are pretty clear about the safety risks.

In that sense, I don't think this is a security bug in Bugzilla's code itself. It might be a security bug on some particular installation that has customized their code, but that's not our code.

However, I agree that you shouldn't be able to set the reporter field in any situation.

Unless you can show me some security risk that this creates in upstream Bugzilla itself, I think we should remove this from the security group. People using email_in.pl should be aware of its security risks.
(Assignee)

Comment 7

9 years ago
(In reply to comment #6)
> I think we should remove this from the security group. People
> using email_in.pl should be aware of its security risks.

That's paradoxal. Should be aware of what? That using email_in.pl in 3.0 and 3.2 is like turning off user authentication completely? I don't see how any bug could have more security risk than that.
(Assignee)

Updated

9 years ago
Flags: approval?
Flags: approval3.0?

Comment 8

9 years ago
(In reply to comment #7)
> Should be aware of what? That using email_in.pl in 3.0 and
> 3.2 is like turning off user authentication completely?

  Yes, because the POD says so:

  http://www.bugzilla.org/docs/3.0/html/api/email_in.html#CAUTION

Updated

9 years ago
Summary: email_in.pl lets you do changes as if you were someone else → email_in.pl lets you set the changer as @reporter instead of only checking the "From" header
(Assignee)

Comment 9

9 years ago
Pushing this bug in our radar for 3.x.4. But has to wait for the release to happen before being committed to CVS.
Flags: approval?
Flags: approval3.0?
Flags: approval3.0+
Flags: approval+
(Assignee)

Updated

9 years ago
Blocks: 417505
(Assignee)

Updated

9 years ago
Blocks: 425110
(Assignee)

Updated

9 years ago
No longer blocks: 417505
(Assignee)

Updated

9 years ago
Whiteboard: [ready for 3.x]
(Assignee)

Comment 10

9 years ago
This problem is present since email_in.pl exists, i.e. 2.23.4.
Summary: email_in.pl lets you set the changer as @reporter instead of only checking the "From" header → [SECURITY] email_in.pl lets you set the changer as @reporter instead of only checking the "From" header
Version: 3.0.3 → 2.23.4
(Assignee)

Comment 11

9 years ago
tip:

Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.18; previous revision: 1.17
done

3.0.3:

Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.5.2.11; previous revision: 1.5.2.10
done
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 12

9 years ago
Security advisory sent, removing bugs from security group.
Group: webtools-security
You need to log in before you can comment on or make changes to this bug.