User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 Adding a simple ACL with (targattrfilters="smth") as target crashes the server Reproducible: Always Steps to Reproduce: 1. Install 389-ds-base-1.2.6-0.3.a3.el5 from rpm 2. setup-ds-admin.pl with express installation options 3. try to add to any entry the following acl: aci: (targattrfilters="description")(version 3.0;acl "Hehe"; allow(read,search,compare) (userdn="ldap:///anyone"); ) Actual Results: The server crashes Expected Results: The server should not crash, some sensible error message should be displayed. The ACL syntax validation procedure should be more robust... Platform: CentOS 5.4 x86_64 The debuginfo for 389-ds-base-1.2.6-0.3.a3.el5 is not available from epel-testing-debuginfo, so the stack trace is not very informative. Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x4684b940 (LWP 26788)] 0x00002b7a1c00e3f2 in acl_parse () from /usr/lib64/dirsrv/plugins/libacl-plugin.so (gdb) bt full #0 0x00002b7a1c00e3f2 in acl_parse () from /usr/lib64/dirsrv/plugins/libacl-plugin.so No symbol table info available. #1 0x00002b7a1c00f010 in acl_verify_syntax () from /usr/lib64/dirsrv/plugins/libacl-plugin.so No symbol table info available. #2 0x00002b7a1bfff9f9 in acl_check_mods () from /usr/lib64/dirsrv/plugins/libacl-plugin.so No symbol table info available. #3 0x00002b7a166289bd in plugin_call_acl_mods_access () from /usr/lib64/dirsrv/libslapd.so.0 No symbol table info available. #4 0x00002b7a1df7559d in ldbm_back_modify () from /usr/lib64/dirsrv/plugins/libback-ldbm.so No symbol table info available. #5 0x00002b7a1661a07a in ?? () from /usr/lib64/dirsrv/libslapd.so.0 No symbol table info available. #6 0x00002b7a1661b0f9 in do_modify () from /usr/lib64/dirsrv/libslapd.so.0 No symbol table info available. #7 0x0000000000412eba in sasl_map_config_add () No symbol table info available. #8 0x00002b7a1743a67d in ?? () from /usr/lib64/libnspr4.so No symbol table info available. #9 0x0000003862206617 in start_thread () from /lib64/libpthread.so.0 No symbol table info available. #10 0x00000038612d3c2d in clone () from /lib64/libc.so.6 No symbol table info available.
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.