Skip to content

CS: Update cs/ldap-injection qhelp #20254

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

Merged
merged 2 commits into from
Aug 21, 2025
Merged

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Aug 20, 2025

Removed broken links and added additional recommendations for improved guidance from cs/ldap-injection qhelp.

Copy link
Contributor

github-actions bot commented Aug 20, 2025

QHelp previews:

csharp/ql/src/Security Features/CWE-090/LDAPInjection.qhelp

LDAP query built from user-controlled sources

If an LDAP query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious LDAP queries.

Recommendation

If user input must be included in an LDAP query, it should be escaped to avoid a malicious user providing special characters that change the meaning of the query. If possible, use an existing library, such as the AntiXSS library. One may also make their own encoder filter `LdapEncode` following RFC 4515 standards.

Example

In the following examples, the code accepts an "organization name" and a "username" from the user, which it uses to query LDAP to access a "type" property.

The first example concatenates the unvalidated and unencoded user input directly into both the DN (Distinguished Name) and the search filter used for the LDAP query. A malicious user could provide special characters to change the meaning of these queries, and search for a completely different set of values.

The second example uses the Microsoft AntiXSS library to encode the user values before they are included in the DN and search filters. This ensures the meaning of the query cannot be changed by a malicious user.

using Microsoft.Security.Application.Encoder
using System;
using System.DirectoryServices;
using System.Web;

public class LDAPInjectionHandler : IHttpHandler
{
    public void ProcessRequest(HttpContext ctx)
    {
        string userName = ctx.Request.QueryString["username"];
        string organizationName = ctx.Request.QueryString["organization_name"];
        // BAD: User input used in DN (Distinguished Name) without encoding
        string ldapQuery = "LDAP://myserver/OU=People,O=" + organizationName;
        using (DirectoryEntry root = new DirectoryEntry(ldapQuery))
        {
            // BAD: User input used in search filter without encoding
            DirectorySearcher ds = new DirectorySearcher(root, "username=" + userName);

            SearchResult result = ds.FindOne();
            if (result != null)
            {
                using (DirectoryEntry user = result.getDirectoryEntry())
                {
                    ctx.Response.Write(user.Properties["type"].Value)
                }
            }
        }

        // GOOD: Organization name is encoded before being used in DN
        string safeOrganizationName = Encoder.LdapDistinguishedNameEncode(organizationName);
        string safeLDAPQuery = "LDAP://myserver/OU=People,O=" + safeOrganizationName;
        using (DirectoryEntry root = new DirectoryEntry(safeLDAPQuery))
        {
            // GOOD: User input is encoded before being used in search filter
            string safeUserName = Encoder.LdapFilterEncode(userName);
            DirectorySearcher ds = new DirectorySearcher(root, "username=" + safeUserName);

            SearchResult result = ds.FindOne();
            if (result != null)
            {
                using (DirectoryEntry user = result.getDirectoryEntry())
                {
                    ctx.Response.Write(user.Properties["type"].Value)
                }
            }
        }
    }
}

References

@Napalys Napalys force-pushed the cs/ldap-injection-qhelp branch 2 times, most recently from 58176d0 to 3432368 Compare August 20, 2025 11:36
@Napalys Napalys force-pushed the cs/ldap-injection-qhelp branch from 3432368 to 71a8e10 Compare August 20, 2025 11:37
@Napalys Napalys marked this pull request as ready for review August 20, 2025 15:27
@Napalys Napalys requested a review from a team as a code owner August 20, 2025 15:27
@Copilot Copilot AI review requested due to automatic review settings August 20, 2025 15:27
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Updates the LDAP injection query help documentation by removing broken AntiXSS library links and adding improved guidance with RFC 4515 standards reference.

  • Removed two broken links to AntiXSS documentation that are no longer accessible
  • Added recommendation to create custom LdapEncode filter following RFC 4515 standards
  • Replaced broken reference links with a valid RFC 4515 reference for LDAP string search filter definition

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@Napalys Napalys added the no-change-note-required This PR does not need a change note label Aug 20, 2025
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Napalys Napalys merged commit 3369e16 into github:main Aug 21, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# documentation no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants