Infra-Mirrors/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2024-10-01 05:36:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

Added proper escaping to LDAP filter operations

Browse Source 
To cover #1163
This commit is contained in:
Dan Brown 2018-12-20 20:04:09 +00:00
parent 7f6929d716
commit 26ec1cc3dc
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
3 changed files with 61 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
app/Auth/Access/Ldap.php
View File

@ -92,4 +92,27 @@ class Ldap
{
return ldap_bind($ldapConnection, $bindRdn, $bindPassword);
}
/**
* Explode a LDAP dn string into an array of components.
* @param string $dn
* @param int $withAttrib
* @return array
*/
public function explodeDn(string $dn, int $withAttrib)
{
return ldap_explode_dn($dn, $withAttrib);
}
/**
* Escape a string for use in an LDAP filter.
* @param string $value
* @param string $ignore
* @param int $flags
* @return string
*/
public function escape(string $value, string $ignore = "", int $flags = 0)
{
return ldap_escape($value, $ignore, $flags);
}
}

19
app/Auth/Access/LdapService.php
View File

@ -107,6 +107,7 @@ class LdapService
if ($ldapUser === null) {
return false;
}
if ($ldapUser['uid'] !== $user->external_auth_id) {
return false;
}
@ -195,7 +196,7 @@ class LdapService
$newAttrs = [];
foreach ($attrs as $key => $attrText) {
$newKey = '${' . $key . '}';
$newAttrs[$newKey] = $attrText;
$newAttrs[$newKey] = $this->ldap->escape($attrText);
}
return strtr($filterString, $newAttrs);
}
@ -265,7 +266,8 @@ class LdapService
$baseDn = $this->config['base_dn'];
$groupsAttr = strtolower($this->config['group_attribute']);
$groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, 'CN='.$groupName, [$groupsAttr]);
$groupFilter = 'CN=' . $this->ldap->escape($groupName);
$groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $groupFilter, [$groupsAttr]);
if ($groups['count'] === 0) {
return [];
}
@ -277,23 +279,26 @@ class LdapService
/**
* Filter out LDAP CN and DN language in a ldap search return
* Gets the base CN (common name) of the string
* @param string $ldapSearchReturn
* @param array $userGroupSearchResponse
* @return array
*/
protected function groupFilter($ldapSearchReturn)
protected function groupFilter(array $userGroupSearchResponse)
{
$groupsAttr = strtolower($this->config['group_attribute']);
$ldapGroups = [];
$count = 0;
if (isset($ldapSearchReturn[$groupsAttr]['count'])) {
$count = (int) $ldapSearchReturn[$groupsAttr]['count'];
if (isset($userGroupSearchResponse[$groupsAttr]['count'])) {
$count = (int) $userGroupSearchResponse[$groupsAttr]['count'];
}
for ($i=0; $i<$count; $i++) {
$dnComponents = ldap_explode_dn($ldapSearchReturn[$groupsAttr][$i], 1);
$dnComponents = $this->ldap->explodeDn($userGroupSearchResponse[$groupsAttr][$i], 1);
if (!in_array($dnComponents[0], $ldapGroups)) {
$ldapGroups[] = $dnComponents[0];
}
}
return $ldapGroups;
}

27
tests/Auth/LdapTest.php
View File

@ -31,6 +31,20 @@ class LdapTest extends BrowserKitTest
$this->mockUser = factory(User::class)->make();
}
protected function mockEscapes($times = 1)
{
$this->mockLdap->shouldReceive('escape')->times($times)->andReturnUsing(function($val) {
return ldap_escape($val);
});
}
protected function mockExplodes($times = 1)
{
$this->mockLdap->shouldReceive('explodeDn')->times($times)->andReturnUsing(function($dn, $withAttrib) {
return ldap_explode_dn($dn, $withAttrib);
});
}
public function test_login()
{
$this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
@ -44,6 +58,7 @@ class LdapTest extends BrowserKitTest
'dn' => ['dc=test' . config('services.ldap.base_dn')]
]]);
$this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
$this->mockEscapes(4);
$this->visit('/login')
->see('Username')
@ -73,6 +88,7 @@ class LdapTest extends BrowserKitTest
'mail' => [$this->mockUser->email]
]]);
$this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true);
$this->mockEscapes(2);
$this->visit('/login')
->see('Username')
@ -97,6 +113,7 @@ class LdapTest extends BrowserKitTest
'dn' => ['dc=test' . config('services.ldap.base_dn')]
]]);
$this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true, true, false);
$this->mockEscapes(2);
$this->visit('/login')
->see('Username')
@ -146,7 +163,7 @@ class LdapTest extends BrowserKitTest
->dontSee('External Authentication');
}
public function test_login_maps_roles_and_retains_existsing_roles()
public function test_login_maps_roles_and_retains_existing_roles()
{
$roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']);
$roleToReceive2 = factory(Role::class)->create(['name' => 'ldaptester-second', 'display_name' => 'LdapTester Second']);
@ -176,6 +193,8 @@ class LdapTest extends BrowserKitTest
]
]]);
$this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
$this->mockEscapes(5);
$this->mockExplodes(6);
$this->visit('/login')
->see('Username')
@ -227,6 +246,8 @@ class LdapTest extends BrowserKitTest
]
]]);
$this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true);
$this->mockEscapes(4);
$this->mockExplodes(2);
$this->visit('/login')
->see('Username')
@ -279,6 +300,8 @@ class LdapTest extends BrowserKitTest
]
]]);
$this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true);
$this->mockEscapes(4);
$this->mockExplodes(2);
$this->visit('/login')
->see('Username')
@ -328,6 +351,8 @@ class LdapTest extends BrowserKitTest
]
]]);
$this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
$this->mockEscapes(5);
$this->mockExplodes(6);
$this->visit('/login')
->see('Username')