This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.3 in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push: new 05749c0 Issue 51054 - AddressSanitizer: heap-buffer-overflow in ldap_utf8prev 05749c0 is described below
commit 05749c04e286434d789aa60eda91d850a7b768e2 Author: Mark Reynolds mreynolds@redhat.com AuthorDate: Mon Apr 27 16:38:31 2020 -0400
Issue 51054 - AddressSanitizer: heap-buffer-overflow in ldap_utf8prev
Bug Description: Adding an invalid/double equal sign when setting the target/targetattr/targetfilter will cause a heap "underflow":
targetfilter=="(uid=*)"
Fix description: Detect and reject these invalid ACI syntaxes before we "underflow". Simply check if the character after the first equal sign is a double quote, as that is the only possible next valid character in a valid ACI.
fixes: https://pagure.io/389-ds-base/issue/51054
Reviewed by: firstyear(Thanks!) --- dirsrvtests/tests/suites/acl/syntax_test.py | 4 ++-- ldap/servers/plugins/acl/aclparse.c | 34 ++++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/dirsrvtests/tests/suites/acl/syntax_test.py b/dirsrvtests/tests/suites/acl/syntax_test.py index 556b19b..cf08c65 100644 --- a/dirsrvtests/tests/suites/acl/syntax_test.py +++ b/dirsrvtests/tests/suites/acl/syntax_test.py @@ -10,10 +10,10 @@
import os import pytest - from lib389._constants import DEFAULT_SUFFIX from lib389.idm.domain import Domain from lib389.topologies import topology_st as topo +from lib389.utils import ds_is_older
import ldap
@@ -192,7 +192,7 @@ FAILED = [('test_targattrfilters_18', f'(all)userdn="ldap:///anyone";)'), ]
-@pytest.mark.xfail(reason='https://bugzilla.redhat.com/show_bug.cgi?id=1691473') +@pytest.mark.skipif(ds_is_older('1.4.3'), reason="Not implemented") @pytest.mark.parametrize("real_value", [a[1] for a in FAILED], ids=[a[0] for a in FAILED]) def test_aci_invalid_syntax_fail(topo, real_value): diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c index e3741f3..d5edaf5 100644 --- a/ldap/servers/plugins/acl/aclparse.c +++ b/ldap/servers/plugins/acl/aclparse.c @@ -309,12 +309,18 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf) tmpstr++; __acl_strip_leading_space(&tmpstr);
+ /* The first character is expected to be a double quote */ + if (*tmpstr != '"') { + slapi_log_err(SLAPI_LOG_ERR, plugin_name, + "__aclp__parse_aci - target filter has an invalid value (%s)\n", str); + return ACL_SYNTAX_ERR; + } + /* * Trim off enclosing quotes and enclosing * superfluous brackets. * The result has been duped so it can be kept. - */ - + */ tmpstr = __acl_trim_filterstr(tmpstr);
f = slapi_str2filter(tmpstr); @@ -323,9 +329,10 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf) aci_item->targetFilterStr = tmpstr;
} else if ((strncmp(str, aci_target_to, target_to_len) == 0) || (strncmp(str, aci_target_from, target_from_len) == 0)) { - /* This is important to make this test before aci_targetdn - * because aci_targetdn also match aci_target_to/aci_target_from - * */ + /* + * This is important to make this test before aci_targetdn + * because aci_targetdn also match aci_target_to/aci_target_from + */ char *tstr = NULL; size_t LDAP_URL_prefix_len = 0; size_t tmplen = 0; @@ -351,6 +358,12 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf) value = s + 1; __acl_strip_leading_space(&value); __acl_strip_trailing_space(value); + /* The first character is expected to be a double quote */ + if (*value != '"') { + slapi_log_err(SLAPI_LOG_ERR, plugin_name, + "__aclp__parse_aci - target to/from has an invalid value (%s)\n", str); + return ACL_SYNTAX_ERR; + } len = strlen(value); /* strip double quotes */ if (*value == '"' && value[len - 1] == '"') { @@ -404,6 +417,12 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf) value = s + 1; __acl_strip_leading_space(&value); __acl_strip_trailing_space(value); + /* The first character is expected to be a double quote */ + if (*value != '"') { + slapi_log_err(SLAPI_LOG_ERR, plugin_name, + "__aclp__parse_aci - target has an invalid value (%s)\n", str); + return ACL_SYNTAX_ERR; + } len = strlen(value); /* strip double quotes */ if (*value == '"' && value[len - 1] == '"') { @@ -1526,6 +1545,11 @@ __aclp__init_targetattr(aci_t *aci, char *attr_val, char **errbuf) return ACL_SYNTAX_ERR; } s++; /* skip leading quote */ + } else { + /* The first character is expected to be a double quote */ + slapi_log_err(SLAPI_LOG_ERR, plugin_name, + "__aclp__init_targetattr - targetattr has an invalid value (%s)\n", attr_val); + return ACL_SYNTAX_ERR; }
str = s;