Closed Bug 785470 (CVE-2012-3981) Opened 12 years ago Closed 12 years ago

[SECURITY] Missing escaping of the username can lead to LDAP injection

Categories

(Bugzilla :: User Accounts, defect)

2.12
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Bugzilla 3.6

People

(Reporter: LpSolit, Assigned: reed)

References

Details

Attachments

(1 file, 1 obsolete file)

When the user enters his credentials, no sanity check is done and his username is passed as is to create the filter which will be passed to $self->ldap->search(), see Bugzilla::Auth::Verify::LDAP::_bz_search_params(). This can lead to LDAP injection, as exploited successfully in bug 785112.

The username must be escaped, e.g. by using Net::LDAP::Util::escape_filter_value().

This problem exists since LDAP authentication has been implemented in Bugzilla 2.12, see bug 51185.
Flags: blocking4.4+
Flags: blocking4.2.3+
Flags: blocking4.0.8+
Flags: blocking3.6.11+
Attached patch patch - v1 (untested) (obsolete) — Splinter Review
something like this? :)
Attachment #655094 - Flags: review?(LpSolit)
Comment on attachment 655094 [details] [diff] [review]
patch - v1 (untested)

>-                      . "=$username)"
>+                      . '=' . escape_filter_value($username) . ')'

Nit: instead of this, we could write:

    $username = escape_filter_value($username);

before building the filter. The code would be easier to parse.


Anyway, I had the same fix in mind, so r=LpSolit from a code point of view, but I'm totally unable to test this patch as I have no LDAP installation to play with. So before being checked in, it needs to be tested (we cannot afford to break LDAP authentication on branches).
Attachment #655094 - Flags: review?(LpSolit)
Attachment #655094 - Flags: review?
Attachment #655094 - Flags: review+
Assignee: user-accounts → reed
Status: NEW → ASSIGNED
CC'ing our LDAP guru. :) manu, could you look at the patch, please?
I've tested the patch and it doesn't break LDAP authentification (mine, at least).
Like LpSolit, I'm far more partiel to adding:

    $username = escape_filter_value($username);

before the return call. It's much more readable that way.
Comment on attachment 655094 [details] [diff] [review]
patch - v1 (untested)

OK, manu's testing is enough for me. No need for another review. Thanks, manu! :)
Attachment #655094 - Flags: review?
reed, we need a CVE number for this one.
Flags: approval?
Flags: approval4.2?
Flags: approval4.0?
Flags: approval3.6?
Summary: [SECURITY] Missing escaping of the username can lead to LDAP injection → (CVE-2012-3981)[SECURITY] Missing escaping of the username can lead to LDAP injection
Alias: CVE-2012-3981
Summary: (CVE-2012-3981)[SECURITY] Missing escaping of the username can lead to LDAP injection → [SECURITY] Missing escaping of the username can lead to LDAP injection
Attached patch patch - v2Splinter Review
For check-in.
Attachment #655094 - Attachment is obsolete: true
Attachment #656030 - Flags: review?(LpSolit)
Attachment #656030 - Attachment is patch: true
I've discussed this a bit with LpSolit and I don't how this allows you to inject data into the LDAP directory.
At best, you can probably invalid the filter that the Bugzilla administrator put in place but that's a stretch and you'll still need the password of an account to login.
Alias: CVE-2012-3981
Alias: CVE-2012-3981
Comment on attachment 656030 [details] [diff] [review]
patch - v2

r=LpSolit
Attachment #656030 - Flags: review?(LpSolit) → review+
Blocks: 786364
Flags: approval?
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Auth/Verify/LDAP.pm
Committed revision 8370.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/Auth/Verify/LDAP.pm
Committed revision 8132.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Auth/Verify/LDAP.pm
Committed revision 7721.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/Auth/Verify/LDAP.pm
Committed revision 7297.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Group: bugzilla-security
You need to log in before you can comment on or make changes to this bug.