As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 148674 - Boolean Charts don't work in Netpositive because '-' is sent as '%2D'
: Boolean Charts don't work in Netpositive because '-' is sent as '%2D'
Status: RESOLVED FIXED
applied to 2.14.2
:
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 2.14.1
: x86 BeOS
: -- normal (vote)
: Bugzilla 2.14
Assigned To: timeless
: default-qa
:
Mentors:
http://bugzilla.mozilla.org/buglist.c...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-06-02 14:20 PDT by timeless
Modified: 2012-12-18 20:46 PST (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
url_decode name (594 bytes, patch)
2002-06-02 14:28 PDT, timeless
mozpreed: review+
justdave: review+
Details | Diff | Splinter Review

Description User image timeless 2002-06-02 14:20:36 PDT
We need to treat this url:
http://bugzilla.mozilla.org/buglist.cgi?&product=Bugzilla&product=Webtools&
field0%2D0%2D0=groupset&type0%2D0%2D0=equals&value0%2D0%2D0=512
the same as we treat this url:
http://bugzilla.mozilla.org/buglist.cgi?&product=Bugzilla&product=Webtools&
field0-0-0=groupset&type0-0-0=equals&value0-0-0=512

User-Agent: Mozilla/3.0 (compatible; NetPositive/2.2)

Be lenient in what you accept.
Comment 1 User image timeless 2002-06-02 14:28:05 PDT
Created attachment 85984 [details] [diff] [review]
url_decode name
Comment 2 User image J. Paul Reed [:preed] 2002-06-02 17:41:44 PDT
Comment on attachment 85984 [details] [diff] [review]
url_decode name

Looks like it should solve the symptoms of the problem.

Also, I looked at the code on the 2.14.1-BRANCH and it has the same problem;
justdave: should this be "wanted for 2.14.2"? This seems low-risk/useful enough
to include there as well.

-jpr
Comment 3 User image Dave Miller [:justdave] (justdave@bugzilla.org) 2002-06-02 18:57:38 PDT
-> patch author
Comment 4 User image Dave Miller [:justdave] (justdave@bugzilla.org) 2002-06-02 19:02:15 PDT
Comment on attachment 85984 [details] [diff] [review]
url_decode name

r= justdave

fix is pretty obvious, and I can't imagine it breaking anything.  I've posted
to the reviewers list regarding the suggestion of including this in 2.14.2.
Comment 5 User image Bradley Baetz (:bbaetz) 2002-06-02 19:05:16 PDT
Are we sure that we can do this? ie is there somewhere where something won't be
escaped, but will look like it is? There shouldn't be, of course, but...

The real fix is to use CGI.pl
Comment 6 User image Dave Miller [:justdave] (justdave@bugzilla.org) 2002-06-02 19:16:06 PDT
But we are using CGI.pl.  In fact, that's what this patch is patching. :)
Comment 7 User image Bradley Baetz (:bbaetz) 2002-06-02 19:22:12 PDT
Err, CGI.pm, rather.

You know, the standard perl module which should handle almost anything we throw
at it.... (Yes, I know we can't use it ATM for various reasons)
Comment 8 User image Dave Miller [:justdave] (justdave@bugzilla.org) 2002-06-02 19:38:54 PDT
For reference, here's the relevant chunk of code from CGI.pm:

sub parse_params {
    my($self,$tosplit) = @_;
    my(@pairs) = split(/[&;]/,$tosplit);
    my($param,$value);
    foreach (@pairs) {
        ($param,$value) = split('=',$_,2);
        $param = unescape($param);
        $value = unescape($value);
        $self->add_parameter($param);
        push (@{$self->{$param}},$value);
    }
}

Notice it does the unescape both on the name and the value.  So we're in the clear.
Comment 9 User image Bradley Baetz (:bbaetz) 2002-06-02 19:44:45 PDT
Fair enough. OK, lets get this into the 2.16 branch and trunk - the general
consensus was that we didn't want this for 2.14.
Comment 10 User image timeless 2002-06-02 19:55:55 PDT
fixed on HEAD and BUGZILLA-2_16-BRANCH
Comment 11 User image Dave Miller [:justdave] (justdave@bugzilla.org) 2002-06-03 16:00:27 PDT
OK, in light of:

http://bugzilla.mozilla.org/show_activity.cgi?id=44642

which timeless brought up on IRC, looks like this qualifies as a security bug
after all....

the group bits are defined in the form as "bit-1", "bit-2", "bit-512" etc.

If a member of one of those groups makes ANY changes to that bug with a browser
that encodes the param names this way, those become "bit%2D1" etc.  And since
the "bit-1" field isn't present in the form, it gets removed, thus making the
bug public without the user's intention to do so.
Comment 12 User image timeless 2002-06-03 16:13:36 PDT
fixed on BUGZILLA-2_14_1-BRANCH
Comment 13 User image Matthew Tuck [:CodeMachine] 2002-06-03 17:11:14 PDT
I don't really mind here, but just having the tag "security" doesn't make it
serious without the "exploitable" tag.  If this was the case, you should also
bring bug #130821 and bug #63018 onto the branch.

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