reports.cgi needs to escape (untrusted) url params

RESOLVED FIXED in Bugzilla 2.14

Status

()

Bugzilla
Bugzilla-General
P3
normal
RESOLVED FIXED
17 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: myk)

Tracking

unspecified
Bugzilla 2.14
Other
Other

Details

(Whiteboard: security, URL)

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
note that reports.cgi has several output modes, so this needs to be fixed for 
all modes.
(Reporter)

Updated

17 years ago
Blocks: 38852
Whiteboard: 2.14
moving to real milestones...
Whiteboard: 2.14
Target Milestone: --- → Bugzilla 2.14
(Assignee)

Comment 2

17 years ago
-> myk, cuz I have a patch for this
Assignee: tara → myk
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

17 years ago
Created attachment 33230 [details] [diff] [review]
validates "product" and "output" form variables
(Assignee)

Updated

17 years ago
Keywords: patch

Comment 4

16 years ago
My biggest problem w/this patch is that if the product is defined, you don't get
the header (because of line 80).  Also, the footers are suppressed.  I realize
that this is mostly by design (the $FORM{'banner'} variable), but I think that
error messages could ignore that flag.

Of course, in theory, these messages should never be seen ;)
(Assignee)

Comment 5

16 years ago
Created attachment 33737 [details] [diff] [review]
updated patch that does it "the right way"
(Assignee)

Comment 6

16 years ago
Created attachment 33740 [details] [diff] [review]
new patch w/changes per Andreas' suggestions in bug 38855
(Assignee)

Comment 7

16 years ago
The patch I just attached reorganizes the code in reports.cgi so headers and
footers display correctly on errors and removes some crufty error checking code
that is no longer necessary.

I added a function to CGI.pl called "DisplayError" that uses the "errorhtml"
parameter to display validation errors.  It works a lot like PuntTryAgain but it
prints HTTP response and HTML headers, and it doesn't stop execution after it
prints the message in order to make the calling code easier to understand, since:

DisplayError("blah") && exit;

    is a lot more descriptive than:

DisplayError("blah");

Comment 8

16 years ago
Code looks good... I ran a few simple reports (on my 7 bugs ;) and everything
worked.  Tested passing the param mentioned in the URL and got the error...

r=jake
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Moving to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
(Reporter)

Updated

14 years ago
Whiteboard: 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.