[security] Possible to add comments to a bug as someone else

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Creating/Changing Bugs
P1
blocker
RESOLVED FIXED
16 years ago
5 years ago

People

(Reporter: bbaetz, Assigned: justdave)

Tracking

2.15
Bugzilla 2.16

Details

(Whiteboard: applied to 2.14.1)

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
[filed in security group, if you're not there, you won't get mail until bmo is
upgraded, and I hope that this is fixed by then, anyway. myk, if you disagree,
then open it up]

So, theres this form element, $::FORM{'who'}. And its used as the "comment added
by" user. Problem is, the only check done is that the user exists in the db (by
quietly_check_login). theres no check that I am that actual user.

Why is this there? Obsolete, or does it have some purpose? I couldn't see where
we ever add an input element with a name of 'who'. If the form element isn't
there, we use the logged in user's id to grab the name.

Its easy to check that the username matches the logged in user, but I wonder if
it may be better to get rid of this stuff entirely. I need to find out why its
there first, though...

A user can manually edit the bugs page to spoof the comment - I'll comment as
nobody@mozilla.org in a sec.

The only other thing which can be done by exploiting this is to change who
processmail thinks did the change, so if excludeSelf is set in the mail prefs,
you can make someone not get mail on a change you make. That ones minor, though.
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.16
Hi, I'm nobody.

Comment 2

16 years ago
My biggest question with this would be, is there any reason to ever need to call
AppendComment() to add a comment as a user other than the currently logged in
user?  If not, I think we need to just use $::userid from within the
AppendComment() routine rather than allow $who to be passed in.

We also need to get rid of the $::FORM{'who'} getting passed on the command line
of processmail.
And note that sice b.m.o hasn't picked up the email/buglist for CC's on
confidential bugs stuff yet, most of the people on the CC list here have never
seen an email about this bug (myself included).  I only found it because it got
mentioned on the b.m.o upgrade bug.
Created attachment 56552 [details] [diff] [review]
Patch v1 - replace all $::FORM{'who'} with $::COOKIE{'Bugzilla_login'}

Comment 5

16 years ago
Comment on attachment 56552 [details] [diff] [review]
Patch v1 - replace all $::FORM{'who'} with $::COOKIE{'Bugzilla_login'}

It's identical to the patch I was about to attach after it was decided that we did
need the ability to add a comment as a user that isn't logged in (for bug moving
amoung other things).

r=jake
Attachment #56552 - Flags: review+

Comment 6

16 years ago
-> Patch author
Assignee: myk → justdave
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.104; previous revision: 1.103
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
This applied to the 2.14.1 branch with no changes.

/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.96.2.1; previous revision: 1.96
Whiteboard: applied to 2.14.1

Comment 9

16 years ago
shouldn't this bug be closed out now?
read the status.  it IS resolved.  I didn't reopen it, I just commented that the
patches had been checked into the 2.14.1 branch.
Testing, ignore this.
Hmm, it seems the bulk change thinks I'm not changing anything if all I do is
add names to the CC list, so I guess I have to make a comment.  Anyhow, adding
the representatives from the organizations we know of that support Bugzilla
distributions so they're aware of our upcoming security release
Opening security bugs for which fixes have appeared in official bugzilla
release.  As per justdave and his posse.
Group: security?
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.