Closed Bug 263780 Opened 20 years ago Closed 20 years ago

xml output format ignores insidergroup and timetrackinggroup, displays private comments

Categories

(Bugzilla :: User Interface, defect, P1)

2.19
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: bugreport, Assigned: bugreport)

Details

(Whiteboard: [does not affect 2.16.x] [fixed in 2.18rc3] [fixed in 2.19.1])

Attachments

(1 file)

show_bug.cgi?id=1&ctype=xml
shows all fields and comments even if not authorized
This needs to be fixed in show_bug.cgi by removing fields from displayfields and
then in the template by checking for private on longdescs and attachments
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.18
Flags: blocking2.20?
Flags: blocking2.18?
Summary: xml output format ignores insidergroup and timetrackinggroup → xml output format ignores insidergroup and timetrackinggroup, displays private comments
Hmm. Is it perhaps instead the right solution to check permissions inside the
CGI and not even make the data available to templates if they shouldn't display it?

That would mean, in terms of comments, that we'd need to pass comment numbers
along with the data, as they wouldn't necessarily be contiguous any more.

Gerv
Attachment #161722 - Flags: review?(vladd)
WRT comment 2:
   To do that, we would have to have Bug.pm and attachment.pm make a lot of
decisions based on Bugzilla->User.  Do we want to do that?
(In reply to comment #4)
> WRT comment 2:
>    To do that, we would have to have Bug.pm and attachment.pm make a lot of
> decisions based on Bugzilla->User.  Do we want to do that?

Yes.  I was just thinking that before I even read Gerv's comment.  Especially
with customizing templates making it so darn easy for an admin (or even a
developer, as in this case) to shoot themselves in the foot.  The CGIs already
decide who can see a secured bug before it even gets displayed.  Bug.pm and
Attachment.pm should do the same with secure fields inside the bug as well.

On that note, however, I'm willing to let a filter fix go in as a security fix
as long as we have a new bug filed to handle the back-end data filtering which
someone plans to actually work on.
The more comprehensive fix probably needs to do the following....
1) GetComments needs to move to Comment.pm (??)
2) Bug.pm, Search.pm, Comment.pm(?), and Attachment.pm all need to be aware of
these restrictions and only return information for the current user UNLESS they
are passed another user object.

Assignee: myk → bugreport
Attachment #161722 - Flags: review?(vladd) → review+
Flags: approval?
Flags: approval2.18?
Comment on attachment 161722 [details] [diff] [review]
Filter out private fields

Make sure this gets flagged as a security patch (blatently) in the commit
message when it gets checked in.  The localizers need to make sure to fix this
in their templates as well, so we need to make sure it stands out in the bonsai
query they run to see what's changed in the templates.
holding approvals for security advisory.

/me goes and adds this one to it, this is more serious than the other two.

(In reply to comment #5)
> On that note, however, I'm willing to let a filter fix go in as a security fix
> as long as we have a new bug filed to handle the back-end data filtering which
> someone plans to actually work on.

Do we have a bug for this yet?

The fact that this will affect localizers that already have 2.18 templates done
is a big reason why this needs to be done on the back-end instead, long-term.
Flags: blocking2.20?
Flags: blocking2.20+
Flags: blocking2.18?
Flags: blocking2.18+
Whiteboard: [does not affect 2.16.x] [ready for 2.18rc3] [ready for 2.19.1]
checked in on trunk:

Checking in show_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v  <--  show_bug.cgi
new revision: 1.31; previous revision: 1.30
done
Checking in template/en/default/bug/show.xml.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.xml.tmpl,v  <--
 show.xml.tmpl
new revision: 1.5; previous revision: 1.4
done

and on 2.18 branch:

Checking in show_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v  <--  show_bug.cgi
new revision: 1.29.2.1; previous revision: 1.29
done
Checking in template/en/default/bug/show.xml.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.xml.tmpl,v  <--
 show.xml.tmpl
new revision: 1.3.2.1; previous revision: 1.3
done
Status: NEW → RESOLVED
Closed: 20 years ago
Flags: approval?
Flags: approval2.18?
Flags: approval2.18+
Flags: approval+
Resolution: --- → FIXED
Whiteboard: [does not affect 2.16.x] [ready for 2.18rc3] [ready for 2.19.1] → [does not affect 2.16.x] [fixed in 2.18rc3] [fixed in 2.19.1]
advisory has posted, clearing security flag
Group: webtools-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.