1191937 - If a bug comment contains a reference to a very large bug ID, the bug is no longer viewable with PostgreSQL

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Frédéric Buclin]

Bugzilla::Bug->_extract_bug_ids() extracts all bug IDs found in comments. If a comment contains a very long bug ID such as bug 12346579812345678913245698, then PostgreSQL crashes:

DBD::Pg::st execute failed: ERROR:  invalid input syntax for integer: "1.23465798123457e+25" [for Statement "SELECT DISTINCT bugs.bug_id, reporter, assigned_to, qa_contact,
                reporter_accessible, cclist_accessible, cc.who,
                bug_group_map.bug_id
           FROM bugs
                LEFT JOIN cc
                          ON cc.bug_id = bugs.bug_id
                             AND cc.who = 1
                LEFT JOIN bug_group_map 
                          ON bugs.bug_id = bug_group_map.bug_id
                             AND bug_group_map.group_id NOT IN (...)
          WHERE bugs.bug_id IN (...)
                AND creation_ts IS NOT NULL "] at Bugzilla/User.pm line 1265.
	Bugzilla::User::_visible_bugs_check_and(Bugzilla::User=HASH(0xa1c4198), ARRAY(0x8d42ba8)) called at Bugzilla/User.pm line 1174
	Bugzilla::User::visible_bugs(Bugzilla::User=HASH(0xa1c4198), ARRAY(0x9c03390)) called at Bugzilla/Bug.pm line 515
	Bugzilla::Bug::preload("Bugzilla::Bug", ARRAY(0x9bf72b0)) called at /var/www/html/bugzilla-pg/show_bug.cgi line 94


So _extract_bug_ids() shouldn't blindly accept all IDs found in comments, but make sure they are smaller than MAX_INT_32.

Comment 1

[Frédéric Buclin]

This issue exists since Bugzilla 4.5.5, see bug 1028795.

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Comment 3

[Gervase Markham [:gerv]]

Is there a more central place we should be doing this check? Are we certain this is the only place that takes arbitrary bug IDs and queries the database with them? What about e.g. search?

Gerv

Comment 4

[Frédéric Buclin]

Search is not a problem, because if a user enters a too big integer (or an invalid integer such as a string), he will at most get a crash. But this doesn't affect other users.

Comment 5

[Gervase Markham [:gerv]]

"he will at most get a crash" - but this bug is also about a crash, according to comment 0.

By "a crash", do you mean different things in the two cases? I assumed that you meant, in comment 0, that the server goes down and needs restarting. Is that right? And in comment 4, you mean that the user gets a Bugzilla error in their browser, but the server stays up?

We used to have a Postgres install on Landfill, but it's not linked from the front page any more. Is it still there?

Gerv

Comment 6

[Frédéric Buclin]

The server never needs to be restarted (and this never happened, AFAIK). By crash, I mean getting an error message as in comment 0. This bug is critical because it affects all users, similar to bug 509220. The bug become unaccessible. A user who gets a similar error when running a query as suggested in comment 3 is not critical, because this only affects the one playing with invalid bug IDs.

Comment 7

[Gervase Markham [:gerv]]

Comment on attachment 8644504 [details] [diff] [review]
patch, v1

Review of attachment 8644504 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense. r=gerv.

Gerv

Comment 8

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   4d8d27d..8ebf35e  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   caaec1d..9810308  5.0 -> 5.0