Overflow in memberof

[389-ds-bot]

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49184

• Created at 2017-03-22 02:23:24 by firstyear (@Firstyear)
• Closed at 2017-04-13 18:39:27 as fixed
• Assigned to tbordaz (@tbordaz)

---

Issue Description

 =================================================================
==3715==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f107957d178 at pc 0x7f108a7f38f8 bp 0x7f107957ce70 sp 0x7f107957ce60
READ of size 4 at 0x7f107957d178 thread T10
    0 0x7f108a7f38f7 in memberof_call_foreach_dn ldap/servers/plugins/memberof/memberof.c:811:0
    1 0x7f108a7f54fb in memberof_del_dn_from_groups ldap/servers/plugins/memberof/memberof.c:661:0
    2 0x7f108a7fee3f in memberof_postop_del ldap/servers/plugins/memberof/memberof.c:593:0
    3 0x7f109b84f1da in plugin_call_func ldap/servers/slapd/plugin.c:2072:0
    4 0x7f109b84f610 in plugin_call_plugins (/usr/lib64/dirsrv/libslapd.so.0+0x1a0610)
    5 0x7f108b354f35 in ldbm_back_delete (/usr/lib64/dirsrv/plugins/libback-ldbm.so+0xf8f35)
    4 0x7f109b78732c in op_shared_delete ldap/servers/slapd/delete.c:331:0
    5 0x7f109b7878a3 in ?? ldap/servers/slapd/delete.c:209:0
    6 0x7f108dd08d78 in dna_update_config_event ldap/servers/plugins/dna/dna.c:1621:0
    7 0x7f109b7bc5e9 in eq_call_all ldap/servers/slapd/eventq.c:283:0
    8 0x7f109b7bc5e9 in eq_loop ldap/servers/slapd/eventq.c:330:0
    9 0x7f1099b1fe8a in PR_Select ??:?
    10 0x7f1099b1fe8a in ?? ??:0
    11 0x7f10994bd36c in start_thread (/lib64/libpthread.so.0+0x736c)
    12 0x7f1098face0e in __GI___clone (/lib64/libc.so.6+0x110e0e)

Address 0x7f107957d178 is located in stack of thread T10 at offset 136 in frame
    0 0x7f108a7f521f in memberof_del_dn_from_groups ldap/servers/plugins/memberof/memberof.c:643:0

  This frame has 3 object(s):
    [32, 36) 'cached'
    [96, 112) 'data'
    [160, 176) 'groupattrs' <== Memory access at offset 136 underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
Thread T10 created by T0 here:
    0 0x7f109c062a1f in pthread_create (/lib64/libasan.so.4+0x37a1f)
    1 0x7f1099b1fb69 in PR_Select ??:?
    2 0x7f1099b1fb69 in ?? ??:0

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-03-22 02:23:53

Metadata Update from @Firstyear:

• Issue assigned to Firstyear

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-03-23 03:08:37

@tbordaz Do you have any ideas on this?

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-03-23 03:08:47

Metadata Update from @Firstyear:

• Custom field reviewstatus adjusted to new
• Custom field type adjusted to defect

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-03-23 10:51:45

I think I found the problem. 'data' is memberof_del_dn_data.

typedef struct _memberof_del_dn_data { char dn; char type; } memberof_del_dn_data;

but in memberof_call_foreach_dn, it is accessed as if it is memberof_get_groups_data typedef struct _memberof_get_groups_data { MemberOfConfig config; Slapi_Value memberdn_val; Slapi_ValueSet groupvals; Slapi_ValueSet group_norm_vals; Slapi_ValueSet **already_seen_ndn_vals; PRBool use_cache; } memberof_get_groups_data;

So my first feeling is that there is a bug but I need to confirm this.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-03-24 00:57:23

Ahhh excellent. Do you want me to assign this to you then? I found this issue with an ASAN build on a freeipa install.

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-03-24 14:27:45

Hi william, Yes please assign it to me. Need to clarify if it is a real issue, impacts and fix it.

thanks

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-03-27 01:48:16

Metadata Update from @Firstyear:

• Issue assigned to tbordaz (was: Firstyear)

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-03-31 18:34:24

A tentative fix. Needs to be tested against CI tests.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-04-03 01:13:42

Codewise, I'm willing to ack this as I understand how the fix works and how the issue arose now. Thanks for your great commit message!

I have not run the tests though. Would you like me to do this?

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-04-13 16:13:12

Metadata Update from @tbordaz:

• Custom field reviewstatus reset (from new)

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-04-13 16:14:31

Rework of the previous patch.. to make it simpler

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-04-13 16:15:13

Metadata Update from @tbordaz:

• Custom field reviewstatus adjusted to review

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2017-04-13 16:19:38

Metadata Update from @mreynolds389:

• Custom field reviewstatus adjusted to ack (was: review)

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-04-13 16:47:29

Thanks Mark for the review

git push origin master

Counting objects: 11, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (11/11), done.
 Writing objects: 100% (11/11), 3.21 KiB | 0 bytes/s, done.
 Total 11 (delta 7), reused 0 (delta 0)
 To ssh://git@pagure.io/389-ds-base.git                                                                                                                            
   4b5aeae..7404525  master -> master

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-04-13 16:47:33

Metadata Update from @tbordaz:

• Custom field reviewstatus adjusted to review (was: ack)

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-04-13 18:39:34

Metadata Update from @tbordaz:

• Issue close_status updated to: fixed
• Issue status updated to: Closed (was: Open)

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-04-18 02:01:29

I'll be sure to test this again soon, thanks for the fix @tbordaz

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2017-06-06 16:54:17

Adjust logging level:

32d46d2..bcbc3e9 master -> master

22f4326..306e217 389-ds-base-1.3.6 -> 389-ds-base-1.3.6