Skip to content
Permalink
Browse files Browse the repository at this point in the history
Escape/encode values when building search filters.
  • Loading branch information
chadlwilson committed Mar 6, 2022
1 parent ef94cd8 commit 87fa7da
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 7 deletions.
9 changes: 5 additions & 4 deletions src/main/java/cd/go/apacheds/ApacheDsLdapClient.java
Expand Up @@ -25,6 +25,7 @@
import org.apache.directory.api.ldap.extras.controls.ppolicy_impl.PasswordPolicyDecorator;
import org.apache.directory.api.ldap.model.entry.Entry;
import org.apache.directory.api.ldap.model.exception.LdapException;
import org.apache.directory.api.ldap.model.filter.FilterEncoder;
import org.apache.directory.api.ldap.model.message.*;
import org.apache.directory.api.ldap.model.name.Dn;
import org.apache.directory.ldap.client.api.LdapConnectionConfig;
Expand Down Expand Up @@ -95,7 +96,7 @@ private PasswordWarning performBind(Dn userDn, String password) throws PasswordE
}

@Override
public <T> List<T> search(final String filter, final Object[] filterArgs, final Mapper<T> mapper, final int maxResultCount) {
public <T> List<T> search(final String filter, final String[] filterArgs, final Mapper<T> mapper, final int maxResultCount) {
final List<T> searchResults = new ArrayList<>();
for (String searchBase : ldapConfiguration.getSearchBases()) {
int resultsToFetch = resultsToFetch(maxResultCount, searchResults.size());
Expand All @@ -109,7 +110,7 @@ public <T> List<T> search(final String filter, final Object[] filterArgs, final
.setScope(SearchScope.SUBTREE)
.addAttributes("*")
.setSizeLimit(resultsToFetch)
.setFilter(format(filter, filterArgs))
.setFilter(FilterEncoder.format(filter, filterArgs))
.setTimeLimit(ldapConfiguration.getSearchTimeout())
.setBase(new Dn(searchBase));

Expand All @@ -124,11 +125,11 @@ public <T> List<T> search(final String filter, final Object[] filterArgs, final

@Override
public void validate() {
final String filter = format(ldapConfiguration.getUserSearchFilter(), "test");
final String filter = FilterEncoder.format(ldapConfiguration.getUserSearchFilter(), "test");
ldapConnectionTemplate.searchFirst(ldapConfiguration.getSearchBases().get(0), filter, SearchScope.SUBTREE, entry -> entry);
}

public List<Entry> search(final String filter, final Object[] filterArgs, final int maxResultCount) {
public List<Entry> search(final String filter, final String[] filterArgs, final int maxResultCount) {
return search(filter, filterArgs, resultWrapper -> (Entry) resultWrapper.getResult(), maxResultCount);
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/cd/go/authentication/ldap/LdapClient.java
Expand Up @@ -24,7 +24,7 @@
public interface LdapClient {
<T> T authenticate(String username, String password, Mapper<T> mapper);

<T> List<T> search(String userSearchFilter, Object[] filterArgs, Mapper<T> mapper, int maxResult);
<T> List<T> search(String userSearchFilter, String[] filterArgs, Mapper<T> mapper, int maxResult);

void validate() throws NamingException;
}
2 changes: 1 addition & 1 deletion src/main/java/cd/go/framework/ldap/JNDILdapClient.java
Expand Up @@ -71,7 +71,7 @@ public <T> T authenticate(String username, String password, Mapper<T> mapper) {
}
}

public <T> List<T> search(String filter, Object[] filterArgs, Mapper<T> mapper, int maxResult) {
public <T> List<T> search(String filter, String[] filterArgs, Mapper<T> mapper, int maxResult) {
List<T> results = new ArrayList<>();
DirContext dirContext = getDirContext(ldapConfiguration, ldapConfiguration.getManagerDn(), ldapConfiguration.getPassword());

Expand Down
Expand Up @@ -38,7 +38,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;

class LdapTest {
class ApacheDsLdapClientTest {

@Test
void shouldBeAbleToSearchUsers() throws ParseException {
Expand All @@ -65,6 +65,26 @@ void shouldBeAbleToSearchUsers() throws ParseException {
assertThat(searchRequest.getTimeLimit()).isEqualTo(10);
}

@Test
void shouldEscapeSearchFilterValues() throws ParseException {
final LdapConfiguration ldapConfiguration = new LdapConfigurationBuilder()
.withSearchTimeout(10)
.withSearchBases("ou=foo,dc=bar")
.build();

final LdapConnectionTemplate ldapConnectionTemplate = mock(LdapConnectionTemplate.class);
final ApacheDsLdapClient ldap = new ApacheDsLdapClient(ldapConfiguration, ldapConnectionTemplate);
final ArgumentCaptor<SearchRequest> argumentCaptor = ArgumentCaptor.forClass(SearchRequestImpl.class);

when(ldapConnectionTemplate.search(argumentCaptor.capture(), ArgumentMatchers.<EntryMapper<Entry>>any())).thenReturn(Collections.singletonList(new DefaultEntry()));

String injectionUserName = "*)(objectclass=*";
ldap.search("(uid={0})", new String[]{injectionUserName}, 1);

assertThat(argumentCaptor.getValue().getFilter())
.isEqualTo(FilterParser.parse("(uid=\\2A\\29\\28objectclass=\\2A)"));
}

@Test
void shouldAbleToFetchResultsFromMultipleSearchBase() {
final LdapConfiguration ldapConfiguration = new LdapConfigurationBuilder()
Expand Down

0 comments on commit 87fa7da

Please sign in to comment.