Closed
Bug 419188
Opened 17 years ago
Closed 17 years ago
[SECURITY] email_in.pl lets you set the changer as @reporter instead of only checking the "From" header
Categories
(Bugzilla :: Incoming Email, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: LpSolit, Assigned: LpSolit)
References
Details
(Whiteboard: [ready for 3.x])
Attachments
(1 file, 1 obsolete file)
1.07 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
CC'ing the Mandriva Bugzilla maintainer as he should be aware of the issue.
Comment 4•17 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•17 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•17 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•17 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•17 years ago
|
Flags: approval?
Flags: approval3.0?
Comment 8•17 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•17 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•17 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•17 years ago
|
Whiteboard: [ready for 3.x]
Assignee | ||
Comment 10•17 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•17 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
Closed: 17 years ago
Resolution: --- → FIXED
Comment 12•17 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.
Description
•