Bugzilla::User->visible_bugs should validate input data

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

4.3.2
Bugzilla 4.4
Bug Flags:
approval +
approval4.4 +

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Bugzilla::User->visible_bugs() passes @check_ids directly into the SQL query without first validating that all items are integers. If a caller passes an alias (or any other string) by accident, then this could lead to SQL injection.

Rather than assuming that the caller does its job correctly and only passes integers, visible_bugs() should call detaint_natural() itself and call ThrowCodeError() if something goes wrong, as this should not happen.
(Assignee)

Updated

5 years ago
Whiteboard: [Good Intro Bug]
(Assignee)

Comment 1

5 years ago
(In reply to Frédéric Buclin from comment #0)
> Bugzilla::User->visible_bugs() passes @check_ids directly into the SQL query
> without first validating that all items are integers. If a caller passes an
> alias (or any other string) by accident, then this could lead to SQL
> injection.

Actually, this is not true. We correctly use placeholders for bug IDs. I misread

  ('?') x @check_ids

But it's still a good idea to validate data to prevent e.g. crashes on PostgreSQL which will complain heavily if a string is used instead of an integer.
Summary: Bugzilla::User->visible_bugs should validate input data to avoid SQL injection → Bugzilla::User->visible_bugs should validate input data
Whiteboard: [Good Intro Bug]
(Assignee)

Comment 2

5 years ago
Created attachment 658138 [details] [diff] [review]
patch, v1
Assignee: general → LpSolit
Status: NEW → ASSIGNED
Attachment #658138 - Flags: review?(glob)
Comment on attachment 658138 [details] [diff] [review]
patch, v1

r=glob by inspection.
Attachment #658138 - Flags: review?(glob) → review+
(Assignee)

Updated

5 years ago
Flags: approval4.4+
Flags: approval+
(Assignee)

Comment 4

5 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/User.pm
Committed revision 8389.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/User.pm
Committed revision 8387.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.