-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Conversation
QHelp previews: csharp/ql/src/Security Features/CWE-090/LDAPInjection.qhelpLDAP query built from user-controlled sourcesIf 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. RecommendationIf 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 ExampleIn 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
|
58176d0
to
3432368
Compare
3432368
to
71a8e10
Compare
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Removed broken links and added additional recommendations for improved guidance from
cs/ldap-injection
qhelp.