libasan read buffer overflow in filtercmp.

[progier389]

Issue Description
libasan exits on a a read buffer overflow in slapi_filter_compare
code review shows that the first key size is used instead of the smallest size

Package Version and Platform:

• Platform: Fedora (and probably all platform and version as it is very old code ...)
• Version - Master branch

Steps to Reproduce
Steps to reproduce the behavior:

1. generate a 389 build with libasan
2. modify the suites/vlv/regression_test.py test (insuring that mapping tree is added after backend creation and that objectclass=* subtree search is performed:

--- a/dirsrvtests/tests/suites/vlv/regression_test.py
+++ b/dirsrvtests/tests/suites/vlv/regression_test.py
@@ -84,8 +84,8 @@ def test_bulk_import_when_the_backend_with_vlv_was_recreated(topology_m2):
     MappingTrees(M2).list()[0].delete()
     Backends(M2).list()[0].delete()
     # Recreate the backend and the VLV index on Master 2.
-    M2.mappingtree.create(DEFAULT_SUFFIX, "userRoot")
     M2.backend.cre-- a/dirsrvtests/tests/suites/vlv/regression_test.py
+++ b/dirsrvtests/tests/suites/vlv/regression_test.py
@@ -84,8 +84,8 @@ def test_bulk_import_when_the_backend_with_vlv_was_recreated(topology_m2):
     MappingTrees(M2).list()[0].delete()
     Backends(M2).list()[0].delete()
     # Recreate the backend and the VLV index on Master 2.
-    M2.mappingtree.create(DEFAULT_SUFFIX, "userRoot")
     M2.backend.create(DEFAULT_SUFFIX, {BACKEND_NAME: "userRoot"})
+    M2.mappingtree.create(DEFAULT_SUFFIX, "userRoot")
     # Recreating vlvSrchDn and vlvIndexDn ate(DEFAULT_SUFFIX, {BACKEND_NAME: "userRoot"})
+    M2.mappingtree.create(DEFAULT_SUFFIX, "userRoot")
     # Recreating vlvSrchDn and vlvIndexDn-- a/dirsrvtests/tests/suites/vlv/regression_test.py
+++ b/dirsrvtests/tests/suites/vlv/regression_test.py
@@ -84,8 +84,8 @@ def test_bulk_import_when_the_backend_with_vlv_was_recreated(topology_m2):
     MappingTrees(M2).list()[0].delete()
     Backends(M2).list()[0].delete()
     # Recreate the backend and the VLV index on Master 2.
-    M2.mappingtree.create(DEFAULT_SUFFIX, "userRoot")
     M2.backend.create(DEFAULT_SUFFIX, {BACKEND_NAME: "userRoot"})
+    M2.mappingtree.create(DEFAULT_SUFFIX, "userRoot")
     # Recreating vlvSrchDn and vlvIndexDn on Master 2.
     vlv_searches.create(
         basedn="cn=userRoot,cn=ldbm database,cn=plugins,cn=config",
@@ -101,6 +101,8 @@ def test_bulk_import_when_the_backend_with_vlv_was_recreated(topology_m2):
     repl.test_replication(M2, M1, 30)
     entries = M2.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(cn=*)")
     assert len(entries) > 0
+    entries = M2.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(objectclass=*)")
+    entries = M2.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(objectclass=*)")

3. Set ASAN options:
   export ASAN_OPTIONS="exitcode=0 redzone=64 quarantine_size_mb=512 symbolize=1 detect_deadlocks=1 log_path=$VARRUN/dirsrv/ns-slapd-$inst.asan"

4 Run py.tests $PWD/regression_test.py

5. See (in output) error: /usr/lib64/python3.8/site-packages/ldap/ldapobject.py:324: SERVER_DOWN

6. See in one of the asan log file:
   ==1359435==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x605000f9c9e9 at pc 0x7f36851a73f0 bp 0x7f36027f8e10 sp 0x7f36027f85b8
   READ of size 12 at 0x605000f9c9e9 thread T16
   #0 0x7f36851a73ef (/lib64/libasan.so.6+0x903ef)
   pre compile and normalize search filter #1 0x7f36851a7b26 in memcmp (/lib64/libasan.so.6+0x90b26)
   If node entries are tombstone'd, subordinate entries fail to get the full DN. #2 0x7f3684ed5b46 in slapi_filter_compare /home/progier/sb/tst1min/tst/source/389-ds-base/ldap/servers/slapd/filtercmp.c:379
   ...

[Firstyear]

Curious what the asan options are exitcode=0 redzone=64 quarantine_size_mb=512 symbolize=1 detect_deadlocks=1. If these are useful, we can set them as default options in lib389 for all tests :)

[progier389]

I just write what I used in my script! 😉 Some are mandatory, and some other are not. Mandatory ones: exitcode=0 define the exit code value when libasan detects a problem. it is mandatory if memory_leak detection is enabled (otherwise the py.tests fails at setup because ns-slapd exit code is 1 (because of memory leak is reported when ns-slapd daemonize) symbolize=1 to get symbol in report instead of addresses Not so useful ones: redzone=64 just define the length of the zone reserved after a memory block (it is a trade off - too big leads to memory outage - too small and some buffer overflow may be ignored) quarantine_size_mb=512 (I forgot what it is exactly, (some resource maximum size)) Irembember that I set it when having massive leaks during initials backend redesign tests detect_deadlocks=1 Would be useful in case of dead lock

…

On Wed, Jan 13, 2021 at 1:08 AM Firstyear ***@***.***> wrote: Curious what the asan options are exitcode=0 redzone=64 quarantine_size_mb=512 symbolize=1 detect_deadlocks=1. If these are useful, we can set them as default options in lib389 for all tests :) — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#4534 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ARLA4LLBM4QGZNHBKKGX7MTSZTP7TANCNFSM4V7TZRSQ> .

-- -- 389 Directory Server Development Team

[Firstyear]

Ahh we also have some of these options in https://github.com/389ds/389-ds-base/blob/master/src/lib389/lib389/__init__.py#L1092 for our test cases. I think we need to leave exitcode=1 in lib389 at least so that CI "fails fast" but the redzone+quarantine may help?

Thanks!

[progier389]

Ok, so we still need to explicitly set up exitcode=0 to run py.tests 
With exitcode=1 I got a failure in setup phase:
E               subprocess.CalledProcessError: Command '['/home/progier/sb/389/tst/ci-install/bin/pwdhash', 'uVQ8RyeV7wtOLiY20FqHmsEXkCf7m6B3xlIqW6PBtTLBT4ItGiiefMC.euRWGo1qY']' returned non-zero exit status 1.

[Firstyear]

do you mind posting the output with exitcode=1 of the pwdhash command manually? Generally we should be fixing these leaks ....

[mreynolds389]

do you mind posting the output with exitcode=1 of the pwdhash command manually? Generally we should be fixing these leaks ....

It's the NSS leak I always see. I run into this every time I try and use asan with CI tests (hence it still fails for me without very intrusive hacking to work around it).

[Firstyear]

@mreynolds389 Okay, if that's the case, I think there is a way in LSAN to make a list of known leaks to ignore. I'll look into this today if I get the chance (but I'm under a pile of mail today :( )

[progier389]

@Firstyear: FYI there is not much information about the leak in ASAN output:
just said that it leaks at some address in some module ! -;)
(my guess (based on Mark remark) is that it is probably in some NSS plugin that get unloaded after use ...)

=================================================================
==2416351==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4097 byte(s) in 1 object(s) allocated from:
#0 0x7f48a571f667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
#1 0x7f48a56b3900 (/lib64/libasan.so.6+0x44900)
#2 0x7f48a0c579ed ()

SUMMARY: AddressSanitizer: 4097 byte(s) leaked in 1 allocation(s).

[vashirov]

@progier389, I think you're hitting #4479 (https://bugzilla.redhat.com/show_bug.cgi?id=1905581)

[Firstyear]

@progier389 I don't have this leak occuring (maybe suse is on a different nspr version, I wouldn't be surprised). You can add suppressions so perhaps:

https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer

leak:<fn name>

LSAN_OPTIONS=suppressions=suppr.txt

The hard part will be in this case since we don't have the symbol name that is leaking, adding the suppression could be tricky. I'm going to assume here that you have all the nss/nspr debug infos installed? If the address of the leak never changes, perhaps try that as the suppression?

[progier389]

Yes, nss debug info are installed (gdb found them). And gdb also shows that what is at the specified address when the leak is detected is not valid code, so IMHO the leak occurs in dynamically loaded code that got unloaded afterwards. And the address changed when I rebuilt the server ...
So I guess that I will stay with exitcode=0 for my test ! -;)

[progier389]

Cherry picked to 1.4.3:
54a7419..111774d master -> upstream/master
ebdf252..e0e1803 389-ds-base-1.4.4 -> 389-ds-base-1.4.4
244ddb8..3334cd5 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

[mreynolds389]

This introduced some compiler warnings, can you please fix them:

../389-ds-base/ldap/servers/slapd/filtercmp.c: In function ‘slapi_filter_compare’:
../389-ds-base/ldap/servers/slapd/filtercmp.c:382:17: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  382 |                 slapi_value_get_string(key1[0])
      |                 ^~~~~~~~~~~~~~~~~~~~~~
../389-ds-base/ldap/servers/slapd/filtercmp.c:386:17: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  386 |                 slapi_value_get_string(key2[0])
      |                 ^~~~~~~~~~~~~~~~~~~~~~
../389-ds-base/ldap/servers/slapd/filtercmp.c:347:9: warning: unused variable ‘cmplen’ [-Wunused-variable]
  347 |     int cmplen;
      |         ^~~~~~

[progier389]

Reopen to add a new PR to fix the compiler warning

[progier389]

Mark fixed the warning the waring in issue #4093 ==> So closing again.