Last Comment Bug 38854 - reports.cgi needs to escape (untrusted) url params
: reports.cgi needs to escape (untrusted) url params
Status: RESOLVED FIXED
security
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: Other Other
: P3 normal (vote)
: Bugzilla 2.14
Assigned To: Myk Melez [:myk] [@mykmelez]
: default-qa
Mentors:
http://bugzilla.mozilla.org/reports.c...
Depends on:
Blocks: 38852
  Show dependency treegraph
 
Reported: 2000-05-10 16:06 PDT by Jesse Ruderman
Modified: 2012-12-18 20:46 PST (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
validates "product" and "output" form variables (1.27 KB, patch)
2001-05-04 14:52 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
updated patch that does it "the right way" (5.90 KB, patch)
2001-05-09 14:01 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
new patch w/changes per Andreas' suggestions in bug 38855 (5.95 KB, patch)
2001-05-09 14:14 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review

Description Jesse Ruderman 2000-05-10 16:06:47 PDT
note that reports.cgi has several output modes, so this needs to be fixed for 
all modes.
Comment 1 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-02-27 18:59:39 PST
moving to real milestones...
Comment 2 Myk Melez [:myk] [@mykmelez] 2001-05-04 14:50:32 PDT
-> myk, cuz I have a patch for this
Comment 3 Myk Melez [:myk] [@mykmelez] 2001-05-04 14:52:52 PDT
Created attachment 33230 [details] [diff] [review]
validates "product" and "output" form variables
Comment 4 Jacob Steenhagen 2001-05-07 16:18:42 PDT
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 ;)
Comment 5 Myk Melez [:myk] [@mykmelez] 2001-05-09 14:01:31 PDT
Created attachment 33737 [details] [diff] [review]
updated patch that does it "the right way"
Comment 6 Myk Melez [:myk] [@mykmelez] 2001-05-09 14:14:14 PDT
Created attachment 33740 [details] [diff] [review]
new patch w/changes per Andreas' suggestions in bug 38855
Comment 7 Myk Melez [:myk] [@mykmelez] 2001-05-09 14:22:56 PDT
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 Jacob Steenhagen 2001-05-09 15:57:08 PDT
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
Comment 9 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-05-09 20:04:35 PDT
Checked in.
Comment 10 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-09-02 23:43:50 PDT
Moving to Bugzilla product

Note You need to log in before you can comment on or make changes to this bug.