Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP Injection #5426

Closed
jorgectf opened this issue Mar 14, 2021 · 3 comments
Closed

LDAP Injection #5426

jorgectf opened this issue Mar 14, 2021 · 3 comments

Comments

@jorgectf
Copy link

jorgectf commented Mar 14, 2021

I've tried to contact the security team through security@redash.io but had no response. I'm attaching the CVE contents for a proper fix.

Suggested description

Redash 8.0.0 is affected by LDAP Injection. There is an information leak through the crafting of special queries, escaping the provided template since the username included in the search filter lacks sanitization.

VulnerabilityType Other

CWE-090: LDAP Injection

Affected Component

/redash/authentication/ldap_auth.py ->
(line 45) ldap_user = auth_ldap_user(request.form["email"], request.form["password"])
(line 71) def auth_ldap_user(username, password)
(line 86) settings.LDAP_SEARCH_TEMPLATE % {"username": username},

Attack Type

Remote

Impact Information Disclosure

true

Attack Vectors

To exploit this vulnerability, the LDAP Authentication must be enabled, and an attacker has to craft a query escaping the template.

Entry Point: ldap_user = auth_ldap_user(request.form["email"], request.form["password"]) (line 45)
Since there is no sanitization when it arrives to the actual search function from LDAP (line 86), an special-crafted query can exfiltrate information.

Reference

https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-2078
https://owasp.org/www-community/attacks/LDAP_Injection
https://portswigger.net/kb/issues/00100500_ldap-injection

@arikfr
Copy link
Member

arikfr commented Mar 19, 2021

Thank you for reporting this and sorry that you didn't get a reply. I will try to check if we can recover the original email and whether we have an issue with the security@ mailbox.

As for the issue: while it's true that the search operator is open to injection, I'm not sure what the attacker might achieve using the injection? I might be misunderstanding something about LDAP, so maybe my question is dumb.

def auth_ldap_user(username, password):
server = Server(settings.LDAP_HOST_URL, use_ssl=settings.LDAP_SSL)
if settings.LDAP_BIND_DN is not None:
conn = Connection(
server,
settings.LDAP_BIND_DN,
password=settings.LDAP_BIND_DN_PASSWORD,
authentication=settings.LDAP_AUTH_METHOD,
auto_bind=True,
)
else:
conn = Connection(server, auto_bind=True)
conn.search(
settings.LDAP_SEARCH_DN,
settings.LDAP_SEARCH_TEMPLATE % {"username": username},
attributes=[settings.LDAP_DISPLAY_NAME_KEY, settings.LDAP_EMAIL_KEY],
)
if len(conn.entries) == 0:
return None
user = conn.entries[0]
if not conn.rebind(user=user.entry_dn, password=password):
return None
return user

The way I understand the LDAP authentication function, it does:

  1. Connect to the LDAP server.
  2. Search for the user.
  3. If user found, rebind with provided password.

If I make an injection, the most that can happen is that I will try to login as a different user. At this point I need to provide the user's password.

If I know the other user's password, I might as well provide their email address - why do I need to make an injection?

@jorgectf
Copy link
Author

Thank you for the reply, there's no problem from my side regarding the email issue, since the vulnerability is not critical.

Regarding your question, an attacker may exfiltrate the attributes assigned in attributes=[settings.LDAP_DISPLAY_NAME_KEY, settings.LDAP_EMAIL_KEY] by concatenating OR search filters. This is a minimal demo for this to make sense.

Having two accounts: 1 - AttributeName: ABC | 2 - AttributeName: DEF

(AttributeName=ABC)
# Returns user 1
(|(AttributeName=ABC)(AttributeName=*))
# Returns both users
(|(AttributeName=ABC)(AttributeName=LETTER*))
# Replace LETTER to leak the user2 AttributeName key by key.

This issue may arise if user2 places first in conn.entries[].

Regarding the CVE description and the impact of the vulnerability, you are totally right, an authentication bypass will never occur (description change already requested), but since the repository is in active development, a fix would prevent more vulnerabilities on this side.

When it comes to patching, I suggest using what this comment suggests.

@jorgectf
Copy link
Author

Closing as it seems to be a wont-fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants