Closed Bug 583690 (CVE-2010-2759) Opened 9 years ago Closed 9 years ago

[SECURITY][PostgreSQL] Bugzilla crashes when viewing a bug if a comment contains 'bug <num>' or 'attachment <num>' where <num> is greater than the max allowed integer

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

2.23.1
defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: slava, Assigned: LpSolit)

References

Details

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; ru; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8 ( .NET CLR 3.5.30729)
Build Identifier: 3.6.1

Crash with error 'value "4294967296" is out of range for type integer ' when I type 'bug 4294967296' in comments for bug. 
Database server is PostgreSQL 8.3.7
Bugzilla version 3.6.1


Reproducible: Always

Steps to Reproduce:
1. Open any existing bug
2. Enter 'bug 4294967296' in comment
3. Save changes
Actual Results:  
undef error - DBD::Pg::db selectrow_hashref failed: ERROR: value "4294967296" is out of range for type integer [for Statement " SELECT alias,assigned_to,bug_file_loc,bug_id,bug_severity,bug_status,cclist_accessible,component_id,delta_ts,estimated_time,everconfirmed,op_sys,priority,product_id,qa_contact,remaining_time,rep_platform,reporter_accessible,resolution,short_desc,status_whiteboard,target_milestone,version,reporter AS reporter_id,TO_CHAR(creation_ts, 'YYYY.MM.DD HH24:MI') AS creation_ts,TO_CHAR(deadline, 'YYYY-MM-DD') AS deadline FROM bugs WHERE bug_id = ?"] at Bugzilla/Object.pm line 85 Bugzilla::Object::_init('Bugzilla::Bug', 4294967296) called at Bugzilla/Object.pm line 53 Bugzilla::Object::new('Bugzilla::Bug', 4294967296) called at Bugzilla/Bug.pm line 294 Bugzilla::Bug::new('undef', 4294967296) called at Bugzilla/Template.pm line 326 Bugzilla::Template::get_bug_link('undef', 'undef', 'HASH(0x973b1f4)') called at Bugzilla/Template.pm line 245 Bugzilla::Template::quoteUrls('bug 4294967296', 'Bugzilla::Bug=HASH(0x915426c)', 'Bugzilla::Comment=HASH(0x94cdab4)') called at Bugzilla/Template.pm line 562 Bugzilla::Template::__ANON__('bug 4294967296') called at template/en/default/bug/comments.html.tmpl line 241 eval {...} called at template/en/default/bug/comments.html.tmpl line 175 Template::Provider::__ANON__('Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Context.pm line 348 eval {...} called at lib/i386-freebsd-64int/Template/Context.pm line 321 Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'a_comment') called at Bugzilla/Template/Context.pm line 45 Bugzilla::Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'a_comment') called at template/en/default/bug/comments.html.tmpl line 130 eval {...} called at template/en/default/bug/comments.html.tmpl line 136 eval {...} called at template/en/default/bug/comments.html.tmpl line 18 Template::Provider::__ANON__('Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Document.pm line 151 eval {...} called at lib/i386-freebsd-64int/Template/Document.pm line 149 Template::Document::process('Template::Document=HASH(0x972e548)', 'Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Context.pm line 351 eval {...} called at lib/i386-freebsd-64int/Template/Context.pm line 321 Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'bug/comments.html.tmpl', 'HASH(0x971a910)') called at Bugzilla/Template/Context.pm line 45 Bugzilla::Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'bug/comments.html.tmpl', 'HASH(0x971a910)') called at template/en/default/bug/edit.html.tmpl line 238 eval {...} called at template/en/default/bug/edit.html.tmpl line 18 Template::Provider::__ANON__('Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Document.pm line 151 eval {...} called at lib/i386-freebsd-64int/Template/Document.pm line 149 Template::Document::process('Template::Document=HASH(0x94b3fcc)', 'Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Context.pm line 351 eval {...} called at lib/i386-freebsd-64int/Template/Context.pm line 321 Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'bug/edit.html.tmpl') called at Bugzilla/Template/Context.pm line 45 Bugzilla::Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'bug/edit.html.tmpl') called at template/en/default/bug/show.html.tmpl line 45 eval {...} called at template/en/default/bug/show.html.tmpl line 18 Template::Provider::__ANON__('Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Document.pm line 151 eval {...} called at lib/i386-freebsd-64int/Template/Document.pm line 149 Template::Document::process('Template::Document=HASH(0x92a9544)', 'Bugzilla::Template::Context=HASH(0x8f6fcfc)') called at lib/i386-freebsd-64int/Template/Context.pm line 351 eval {...} called at lib/i386-freebsd-64int/Template/Context.pm line 321 Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'Template::Document=HASH(0x92a9544)') called at Bugzilla/Template/Context.pm line 45 Bugzilla::Template::Context::process('Bugzilla::Template::Context=HASH(0x8f6fcfc)', 'Template::Document=HASH(0x92a9544)') called at lib/i386-freebsd-64int/Template/Service.pm line 94 eval {...} called at lib/i386-freebsd-64int/Template/Service.pm line 91 Template::Service::process('Template::Service=HASH(0x8ef5f7c)', 'bug/show.html.tmpl', 'HASH(0x804d16c)') called at lib/i386-freebsd-64int/Template.pm line 66 Template::process('Bugzilla::Template=HASH(0x8e6f028)', 'bug/show.html.tmpl', 'HASH(0x804d16c)') called at /usr/local/www/bugzilla/show_bug.cgi line 128
Confirmed. MySQL silently returns no result while Pg throws the error reported in comment 0 (no idea about Oracle). This problem is also visible for attachment IDs being too large. You could use this trick to prevent access to all bug reports as you could comment in all bugs, making Bugzilla to crash every time you try to access a bug.
Assignee: create-and-change → LpSolit
Group: bugzilla-security
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: blocking4.0+
Flags: blocking3.6.2+
Flags: blocking3.4.8+
Flags: blocking3.2.8+
OS: Windows Vista → All
Hardware: x86 → All
Summary: Crash if I type 'bug 4294967296' in comments for bug → [SECURITY][PostgreSQL] Bugzilla crashes when viewing a bug if a comment contains 'bug <num>' or 'attachment <num>' where <num> is greater than the max allowed integer
Target Milestone: --- → Bugzilla 3.2
Version: unspecified → 3.6.1
Attached patch patch for 3.6 - 4.1, v1 (obsolete) — Splinter Review
Technically, the constant could be named MAX_INTEGER as Pg makes no distinction between INT3 (mediumint) and INT4 (integer) types. If we rename it MAX_INTEGER, then it could go as a constant in Constants.pm, with value 2147483647, as it would be the same for both MySQL and PostgreSQL (I think Oracle doesn't care), and MySQL won't crash with a number between 8388607 and 2147483647.
Attachment #462035 - Flags: review?(mkanat)
Attached patch patch for 3.6 - 4.1, v1.1 (obsolete) — Splinter Review
I just removed a useless empty line.
Attachment #462035 - Attachment is obsolete: true
Attachment #462037 - Flags: review?(mkanat)
Attachment #462035 - Flags: review?(mkanat)
May be this error must be processed in contructor of "Bugzilla::Bug" class?

Try this: /show_bug.cgi?id=4294967296

DBD::Pg::db selectrow_hashref failed: ERROR: value "4294967296" is out of range for type integer [for Statement " SELECT alias,assigned_to,bug_file_loc,bug_id,bug_severity,bug_status,cclist_accessible,component_id,delta_ts,estimated_time,everconfirmed,op_sys,priority,product_id,qa_contact,remaining_time,rep_platform,reporter_accessible,resolution,short_desc,status_whiteboard,target_milestone,version,reporter AS reporter_id,TO_CHAR(creation_ts, 'YYYY.MM.DD HH24:MI') AS creation_ts,TO_CHAR(deadline, 'YYYY-MM-DD') AS deadline FROM bugs WHERE bug_id = ?"] at Bugzilla/Object.pm line 85 Bugzilla::Object::_init('Bugzilla::Bug', 4294967296) called at Bugzilla/Object.pm line 53 Bugzilla::Object::new('Bugzilla::Bug', 4294967296) called at Bugzilla/Bug.pm line 294 Bugzilla::Bug::new(undef, 4294967296) called at Bugzilla/Bug.pm line 323 Bugzilla::Bug::check('Bugzilla::Bug', 4294967296) called at /usr/local/www/data/bugzilla32/show_bug.cgi line 65
(In reply to comment #4)
> May be this error must be processed in contructor of "Bugzilla::Bug" class?

Well, all places where an ID is passed to the new() method, not only Bugzilla::Bug, but also Bugzilla::Group, Bugzilla::User, etc... could be concerned by this problem. We could put a single check in Bugzilla::Object->new(), but the problem would be to know what to do when a too large integer is passed. Throw an error or fail silently? In the problem described in this bug, you don't want to throw an error, while in some other cases, such as the one you mentioned in comment 4, it may be fine to throw an error telling the user that the ID is too large. But that's definitely out of the scope of this bug.
One more thing: the testcase you describe in comment 4 cannot be used to "DoS" Bugzilla, while you can with the testcase described in comment 0 (which is why I marked it as a security bug).
Ок. But constructor of bug class already have some checking code. And if you try to request bug with invalid or float point id you will get error 
----
            # Aliases are off, and we got something that's not a number.
            my $error_self = {};
            bless $error_self, $class;
            $error_self->{'bug_id'} = $param;
            $error_self->{'error'}  = 'InvalidBugId';
            return $error_self;
----
Comment on attachment 462037 [details] [diff] [review]
patch for 3.6 - 4.1, v1.1

The check needs to go into Bugzilla::Object->new, I'm pretty sure (at least on trunk). Perhaps for the branches, it should just go into Bugzilla::Bug->new and Bugzilla::Attachment->new.
Attachment #462037 - Flags: review?(mkanat) → review-
Also, I think we should target this for 3.6.3 instead--we already have too many security bugs going into 3.6.2.
Also, I suspect this only happens on Pg 8.3 and higher.
Also, new() should return undef in this case--that's what it normally does for invalid values. Just act like somebody asked for an object that doesn't exist.

new_from_list should just remove those values from the input list before running the SQL.
Comment on attachment 462037 [details] [diff] [review]
patch for 3.6 - 4.1, v1.1

No, as I said, this bug is not about fixing new(). We don't want to throw an error, but simply return the string which was passed. Fixing new() is another bug. Also, we should take it for 3.6.2. It's safe and non-invasive.
Attachment #462037 - Flags: review- → review?(mkanat)
Also, fixing new() may have unexpected side-effects I don't want for older branches.
Well, I get what you're saying. However, fixing new() won't have any unexpected side effects. The behavior of "new" is to return "undef" when given an id that does not exist. Since ids greater than the integer maximum can't possibly exist in the database, you are simply correctly returning "undef" for an object that doesn't exist.
Also, since only Pg has the problem, I agree that just having a MAX_INT constant would be fine. Maybe MAX_INT_32 would be the most appropriate name, since it's a 32-bit integer. We could just check things against the MAX_INT constant.
Although yes, for the branches it's hard to say if you'd run into funny things having to do with invalid input--perhaps we should only check it once we've verified that it's an integer (which I suppose we already do, in new()).
Fix new() and new_from_list(). check() calls new(), so it's automatically fixed too.
Attachment #462037 - Attachment is obsolete: true
Attachment #462281 - Flags: review?(mkanat)
Attachment #462037 - Flags: review?(mkanat)
Comment on attachment 462281 [details] [diff] [review]
patch for 3.6-4.1, v2

This looks good. The Object changes I feel pretty confident about and I think they can go on the branches.

I'm a little nervous about modifying get_attachment_link on the 3.6 branch or any earlier. When did we actually start supporting Pg 8.3--I think it was 3.6, yeah? Maybe it was 3.4. Not sure.
Attachment #462281 - Flags: review?(mkanat) → review+
Flags: approval?
Flags: approval4.0?
(In reply to comment #18)
> I'm a little nervous about modifying get_attachment_link on the 3.6 branch or
> any earlier.

Why? I'm personally not.
Mmm, I don't know the performance impact of getting an object (even though it's probably not big), and it seems like a lot of code being changed in a somewhat-critical area.

For the branches I think we should just put the "if $id > MAX_INT_32" thing inside of get_attachment_link--that's a smaller change that accomplishes the same thing, no?
Oh, there is no impact for 3.4 and newer, as I refactored Attachment.pm to use Object.pm, see bug 388251. So you also have a single SQL query. For 3.2, we still use Bugzilla::Attachment->get(), but even there, it's a single SQL query.
Hmm. I see what you're saying. Might it be good to have a smaller patch for the branches anyway, to conflict less with people's customizations and so forth?
Also, does Bugzilla 3.2 even support Pg 8.3? I don't think it does. And this problem almost surely won't happen on any Pg before 8.3.
(In reply to comment #23)
> Also, does Bugzilla 3.2 even support Pg 8.3?

Sure it does. I'm running QA tests using Pg 8.4.4 on all branches, including 3.2.
Okay. Well, I'd be more comfortable with a smaller patch for the branches, which would probably also be good since then one patch would apply to all of them, most likely.
Attachment 462281 [details] [diff] applies correctly on the 3.6 branch too. No reason not to take it there, as we are going to support this branch for a long time, and using an attachment object here is going to save a lot of trouble in the future.

For 3.4 and 3.2, I will attach another patch which doesn't use the attachment object, to minimize changes in Template.pm.
Flags: approval3.6?
Attachment #462281 - Attachment description: patch, v2 → patch for 3.6-4.1, v2
The patch for 4.1 doesn't apply to 3.4 as get_bug_link() was not using a bug object but calls the DB directly. So I had to write another patch for it anyway, which also applies to 3.2, and works.
Attachment #462372 - Flags: review?(mkanat)
Blocks: 580214
Alias: CVE-2010-2759
Fun, Bugzilla 2.20.7 and 2.22.7 work fine with Pg 8.4.4, and are not vulnerable to this bug, because GetBugLink() in globals.pl injected the bug ID and the attachment ID directly in the SQL queries, without placeholders. But Bugzilla 2.23.1 is vulnerable as we removed globals.pl and started using placeholders everywhere.
Version: 3.6.1 → 2.23.1
Comment on attachment 462372 [details] [diff] [review]
patch for 3.2-3.4, v1

Looks good. :-) I think the plan to do this one on 3.4 + 3.2 and the above patch on the rest is a good plan.
Attachment #462372 - Flags: review?(mkanat) → review+
Flags: approval3.4?
Flags: approval3.2?
Flags: approval?
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval3.4?
Flags: approval3.4+
Flags: approval3.2?
Flags: approval3.2+
Flags: approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Constants.pm
modified Bugzilla/Object.pm
modified Bugzilla/Template.pm
Committed revision 7430.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Constants.pm
modified Bugzilla/Object.pm
modified Bugzilla/Template.pm
Committed revision 7371.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/Constants.pm
modified Bugzilla/Object.pm
modified Bugzilla/Template.pm
Committed revision 7160.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.4/
modified Bugzilla/Constants.pm
modified Bugzilla/Object.pm
modified Bugzilla/Template.pm
Committed revision 6774.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.2/
modified Bugzilla/Constants.pm
modified Bugzilla/Object.pm
modified Bugzilla/Template.pm
Committed revision 6395.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Security advisory sent, unlocking bug.
Group: bugzilla-security
You need to log in before you can comment on or make changes to this bug.