Closed Bug 244272 Opened 16 years ago Closed 15 years ago

editusers 'query' parameter should be removed

Categories

(Bugzilla :: Administration, task)

2.17.7
task
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: glob, Assigned: glob)

Details

(Whiteboard: [fixed in 2.16.6] [fixed in 2.18rc1])

Attachments

(2 files, 1 obsolete file)

editusers.cgi, line 322:

  } elsif (exists $::FORM{'query'}) {
    $query = "SELECT login_name,realname,disabledtext " .
        "FROM profiles WHERE " . $::FORM{'query'} . " ORDER BY login_name";

i think this should be removed because:

 (1) i can't find anywhere where it's used
 (2) it's not taint-safe, and can't be untainted without breaking the function
 (3) it's just a bad idea :)
Assignee: justdave → bugzilla
Status: NEW → ASSIGNED
Attached patch remove 'query' (obsolete) — Splinter Review
Attachment #149034 - Flags: review?
Severity: major → normal
Incidentally, the field was originally used from bug 25010 in a link from
editgroups during group deletion.  editgroups no linger uses that parameter.
Comment on attachment 149034 [details] [diff] [review]
remove 'query'

r=jouni
Attachment #149034 - Flags: review? → review+
We'll probably want this for 2.18 since we're toying with the taint stuff anyway?
Flags: approval?
Target Milestone: --- → Bugzilla 2.18
eeewwww....  I remember writing this code.  What the hell was I thinking?

This really needs to go away on the 2.16 branch as well.  It's kind of a
security problem (arbitrary SQL injection) even if it is only exploitable if you
have privs.  Easiest thing to do for 2.16 is probably to backport the group
parameter.
Group: webtools-security
Flags: blocking2.18+
Flags: blocking2.16.6+
Target Milestone: Bugzilla 2.18 → Bugzilla 2.16
Attached patch 2.16 patch, v1Splinter Review
Attachment #149116 - Flags: review?(justdave)
OS: Windows XP → All
Hardware: PC → All
Attachment #149116 - Flags: review?(justdave) → review+
Flags: approval2.16?
Whiteboard: [ready for 2.16.6] [ready for 2.18rc1]
This is the same as attachmnet 149034, but de-bitrotten
Attachment #149034 - Attachment is obsolete: true
Attachment #152376 - Flags: review?(justdave)
Flags: approval2.16? → approval2.16+
Attachment #152376 - Flags: review?(justdave) → review+
Flags: approval? → approval+
checked in on both branches
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [ready for 2.16.6] [ready for 2.18rc1] → [fixed in 2.16.6] [fixed in 2.18rc1]
Clearing the security flag on disclosed bugs
Group: webtools-security
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.