Bug 585905
Summary: | ACL with targattrfilters error crashes the server | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Retired] 389 | Reporter: | Andrey Ivanov <andrey.ivanov> | ||||||
Component: | Security - Access Control (ACL) | Assignee: | Noriko Hosoi <nhosoi> | ||||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Viktor Ashirov <vashirov> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | low | ||||||||
Version: | 1.2.6 | CC: | amsharma, nhosoi, rmeggins | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | x86_64 | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2015-12-07 16:55:33 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 434914, 543590, 639035 | ||||||||
Attachments: |
|
Description
Andrey Ivanov
2010-04-26 11:34:29 UTC
Created attachment 409325 [details]
git patch file (master)
Bug Description:
targattrfilters takes this format of value:
(targattrfilters="add=attr1:F1 && attr2:F2... &&
attrn:Fn,del=attr1:F1 && attr2:F2 ... && attrn:Fn")
The ACL plugin code had blindly expected the value contains
the operator "add" or "del" and '=' to concatenate the
attribute and filter pair. The plugin should have checked
the possibility that the value does not follow the format.
Fix Description:
If '=' is not included in the targattrfilters value, the
ACL parser returns ACL_SYNTAX_ERR.
File: ldap/servers/plugins/acl/aclparse.c
Comment on attachment 409325 [details]
git patch file (master)
first diff section - lines 291- - should be NULL == tempstr
Also, do the gotos or returns potentially cause any memory leaks?
Comment on attachment 409325 [details]
git patch file (master)
sorry - I meant tmpstr not tempstr
(In reply to comment #2) > (From update of attachment 409325 [details]) > first diff section - lines 291- - should be NULL == tempstr > > Also, do the gotos or returns potentially cause any memory leaks? (In reply to comment #3) > (From update of attachment 409325 [details]) > sorry - I meant tmpstr not tempstr Oops. Thanks, Rich! I fixed it. I went through the gotos and returns. I don't think I introduced any "new" leaks. I tested the server with valgrind for the test case that Andrey provided. It does not cause memory leak. Created attachment 409357 [details]
git patch file (master)
Fixed the bug Rich found out. (Thanks a lot!)
Reviewed by Rich and Nathan (Thank you!!). Pushed to master. $ git merge work Updating 3155d9c..b65b3c9 Fast forward ldap/servers/plugins/acl/aclparse.c | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-) $ git push Counting objects: 13, done. Delta compression using 4 threads. Compressing objects: 100% (7/7), done. Writing objects: 100% (7/7), 1.08 KiB, done. Total 7 (delta 5), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git 3155d9c..b65b3c9 master -> master Finally, special thanks to Andrey for providing us this good test case! [root@snmaptest ~]# rpm -qa | grep 389
389-adminutil-1.1.14-1.el6.x86_64
389-adminutil-devel-1.1.14-1.el6.x86_64
389-ds-base-libs-1.2.9.11-1.el6.x86_64
389-ds-console-1.2.6-1.el6.noarch
389-ds-base-debuginfo-1.2.8.2-1.el6_1.3.x86_64
389-console-1.1.7-1.el6.noarch
389-admin-console-1.1.8-1.el6.noarch
389-ds-console-doc-1.2.6-1.el6.noarch
389-ds-1.2.1-2.el6.noarch
389-ds-base-devel-1.2.9.11-1.el6.x86_64
389-ds-base-1.2.9.11-1.el6.x86_64
389-admin-1.1.23-2.el6.x86_64
389-admin-console-doc-1.1.8-1.el6.noarch
[root@snmaptest ~]# ldapmodify -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF
> dn: dc=example,dc=com
> changetype: modify
> replace: aci
> aci: (targattrfilters="description")(version 3.0;acl "Hehe"; allow(read,search,compare) (userdn="ldap:///cn=Userami,dc=test,dc=com"); )
> EOF
modifying entry "dc=example,dc=com"
ldap_modify: Invalid syntax (21)
additional info: ACL Syntax Error(-5):(targattrfilters=\22description\22)(version 3.0;acl \22Hehe\22; allow(read,search,compare) (userdn=\22ldap:///cn=Userami,dc=test,dc=com\22); )
hence VERIFIED.
|