Closed
Bug 98146
Opened 23 years ago
Closed 23 years ago
[security] Minor problem w/ error message if some sort of login error in doeditvotes
Categories
(Bugzilla :: Creating/Changing Bugs, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: bugzilla, Assigned: gerv)
Details
(Whiteboard: security: untrusted var in HTML | applied to 2.14.1)
Attachments
(3 files, 1 obsolete file)
810 bytes,
patch
|
myk
:
review+
myk
:
review+
|
Details | Diff | Splinter Review |
591 bytes,
patch
|
Details | Diff | Splinter Review | |
782 bytes,
patch
|
Details | Diff | Splinter Review |
doeditvotes.cgi contains the following: > if ( (! defined $who) || (!$who) ) { > PutHeader("Bad login."); > > print "The login info got confused. If you want to adjust the votes\n"; > print "for <tt>$::COOKIE{'Bugzilla_login'}</tt>, then please\n"; > print "<a href=showvotes.cgi?user=$who>click here</a>.<hr>\n"; > > PutFooter(); > > exit(); >} If something went mad, and $who was undefined, then this warning message would try and use it in the showvotes.cgi link ( => http log warning + bad link generated) Should the message just say 'please log in (again) and try again', or something like that?
Comment 1•23 years ago
|
||
my first though on this is "oh, it didn't get escaped" but after looking at it for a minute, I agree with Gavin's thoughts on how to fix it. If we thing the login info is screwed up in any way, we shouldn't be displaying what we think is screwed up to the user, in case it's not really them and someone trying to use a faked cookie or something.
Assignee: justdave → myk
Severity: minor → critical
Component: Bugzilla-General → Creating/Changing Bugs
Priority: -- → P1
Whiteboard: security: untrusted var in HTML
Target Milestone: --- → Bugzilla 2.16
Assignee | ||
Comment 2•23 years ago
|
||
Assignee | ||
Comment 3•23 years ago
|
||
1 down, 352 to go... Gerv
Comment 4•23 years ago
|
||
Comment on attachment 53513 [details] [diff] [review] Patch v.1 >+ print "The login info got confused. Please log in (again) and try again.\n"; Shouldn't "log in (again)" be a link?
Attachment #53513 -
Flags: review-
Assignee | ||
Comment 5•23 years ago
|
||
Assignee | ||
Comment 6•23 years ago
|
||
myk: re-review? Gerv
Comment 7•23 years ago
|
||
Comment on attachment 53658 [details] [diff] [review] Patch v.2 >Index: doeditvotes.cgi >=================================================================== >RCS file: /cvsroot/mozilla/webtools/bugzilla/doeditvotes.cgi,v >retrieving revision 1.12 >diff -u -r1.12 doeditvotes.cgi >--- doeditvotes.cgi 2001/08/28 17:55:14 1.12 >+++ doeditvotes.cgi 2001/10/15 22:23:31 >@@ -84,9 +84,11 @@ > > if ( (! defined $who) || (!$who) ) { > PutHeader("Bad login."); >- print "The login info got confused. If you want to adjust the votes\n"; >- print "for <tt>$::COOKIE{'Bugzilla_login'}</tt>, then please\n"; >- print "<a href=showvotes.cgi?user=$who>click here</a>.<hr>\n"; >+ print qq| >+ The login info got confused. Please >+ <a href="http://bugzilla.mozilla.org/query.cgi?GoAheadAndLogIn=1">log in</a> >+ (again) and try again.\n >+ |; > PutFooter(); > exit(); > } Looks good. I can't reproduce the problem, but the fix is trivial, the script works as it should, and tests pass. r=myk, no second review needed. Patch checked in.
Attachment #53658 -
Flags: review+
Comment 8•23 years ago
|
||
Checking in doeditvotes.cgi; /cvsroot/mozilla/webtools/bugzilla/doeditvotes.cgi,v <-- doeditvotes.cgi new revision: 1.13; previous revision: 1.12 done Patch checked in. Resolving fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 9•23 years ago
|
||
Should the fix really have a URL prefixed with "http://bugzilla.mozilla.org/"? That would make all installations point back to bmo, which (I believe) is incorrect behavior. I'd reopen this bug, but I don't have permission to do so.
Comment 11•23 years ago
|
||
Assignee | ||
Comment 12•23 years ago
|
||
Good catch. /cvsroot/mozilla/webtools/bugzilla/doeditvotes.cgi,v <-- doeditvotes.cgi new revision: 1.14; previous revision: 1.13 Gerv
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Comment 13•23 years ago
|
||
Patch (to be attached momentarily) applied to the 2.14.1 branch /cvsroot/mozilla/webtools/bugzilla/doeditvotes.cgi,v <-- doeditvotes.cgi new revision: 1.12.2.1; previous revision: 1.12
Whiteboard: security: untrusted var in HTML → security: untrusted var in HTML | applied to 2.14.1
Comment 14•23 years ago
|
||
Attachment #53513 -
Attachment is obsolete: true
Updated•23 years ago
|
Summary: Minor problem w/ error message if some sort of login error in doeditvotes → [security] Minor problem w/ error message if some sort of login error in doeditvotes
Comment 15•23 years ago
|
||
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
Updated•11 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•