[389-commits] [389-ds-base] branch 389-ds-base-1.4.3 updated: Issue 51054 - AddressSanitizer: heap-buffer-overflow in ldap_utf8prev

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;

--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
389-commits mailing list -- 389-commits@lists.fedoraproject.org
To unsubscribe send an email to 389-commits-leave@lists.fedoraproject.org
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/389-commits@lists.fedoraproject.org