244272 - editusers 'query' parameter should be removed

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[:glob ✱]

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 :)

Comment 1

[:glob ✱]

Attachment: remove 'query'

Comment 2

[Joel Peshkin]

Incidentally, the field was originally used from bug 25010 in a link from
editgroups during group deletion.  editgroups no linger uses that parameter.

Comment 3

[Jouni Heikniemi]

Comment on attachment 149034 [details] [diff] [review]
remove 'query'

r=jouni

Comment 4

[Jouni Heikniemi]

We'll probably want this for 2.18 since we're toying with the taint stuff anyway?

Comment 5

[Dave Miller [:justdave]]

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.

Comment 6

[Jouni Heikniemi]

Attachment: 2.16 patch, v1

Comment 7

[Joel Peshkin]

Attachment: de-bitrotted version

This is the same as attachmnet 149034, but de-bitrotten

Comment 8

[Joel Peshkin]

checked in on both branches

Comment 9

[Dave Miller [:justdave]]

Clearing the security flag on disclosed bugs