net-ads-search crashes when tokengroups attribute is requested

[Isaac Boukris]

On Mon, Jan 22, 2018 at 9:11 PM, Jeremy Allison <jra at samba.org> wrote:
> On Mon, Jan 22, 2018 at 01:00:31PM +0200, Isaac Boukris via samba-technical wrote:
>> I guess this constructed attribute isn't supported, though it still
>> shouldn't crash (git master).
>
> Can you run this under valgrind and make it crash ? I'd be really interested
> to see the output from that.

Right! with valgrind it doesn't crash but it gives good insight.
See full output here:
https://pastebin.com/VkUckwi5

In short:
[root at fs ~]# valgrind  /usr/local/samba/bin/net ads search CN=isaac
tokengroups -P
...
ads_search_retry: failed to reconnect (No logon servers are currently
available to service the logon request.)
search failed: No logon servers are currently available to service the
logon request.
==17262== Invalid read of size 4
==17262==    at 0x67710F6: ads_destroy (in
/usr/local/samba/lib/private/libads-samba4.so)
==17262==    by 0x148DC3: net_ads_search (in /usr/local/samba/bin/net)
==17262==    by 0x1899BC: net_run_function (in /usr/local/samba/bin/net)
==17262==    by 0x14AE6E: net_ads (in /usr/local/samba/bin/net)
==17262==    by 0x1899BC: net_run_function (in /usr/local/samba/bin/net)
==17262==    by 0x1414EF: main (in /usr/local/samba/bin/net)
==17262==  Address 0x1c4249b0 is 0 bytes inside a block of size 448 free'd
==17262==    at 0x4C2FD18: free (vg_replace_malloc.c:530)
==17262==    by 0x6771478: ads_destroy (in
/usr/local/samba/lib/private/libads-samba4.so)
==17262==    by 0x6774348: ads_do_search_retry_internal (in
/usr/local/samba/lib/private/libads-samba4.so)
==17262==    by 0x67745E5: ads_do_search_retry (in
/usr/local/samba/lib/private/libads-samba4.so)
==17262==    by 0x148D44: net_ads_search (in /usr/local/samba/bin/net)
==17262==    by 0x1899BC: net_run_function (in /usr/local/samba/bin/net)
==17262==    by 0x14AE6E: net_ads (in /usr/local/samba/bin/net)
==17262==    by 0x1899BC: net_run_function (in /usr/local/samba/bin/net)
==17262==    by 0x1414EF: main (in /usr/local/samba/bin/net)
==17262==  Block was alloc'd at
==17262==    at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
==17262==    by 0x8474024: smb_xmalloc_array (in
/usr/local/samba/lib/libsmbconf.so.0)
==17262==    by 0x6770FCD: ads_init (in
/usr/local/samba/lib/private/libads-samba4.so)
==17262==    by 0x1422EF: ads_startup_int (in /usr/local/samba/bin/net)
==17262==    by 0x142705: ads_startup (in /usr/local/samba/bin/net)
==17262==    by 0x148CDA: net_ads_search (in /usr/local/samba/bin/net)
==17262==    by 0x1899BC: net_run_function (in /usr/local/samba/bin/net)
==17262==    by 0x14AE6E: net_ads (in /usr/local/samba/bin/net)
==17262==    by 0x1899BC: net_run_function (in /usr/local/samba/bin/net)
==17262==    by 0x1414EF: main (in /usr/local/samba/bin/net)
==17262==


It is still unclear how it is supposed to be, but the below solves the
invalid-read and the crash too:

$ git diff
diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
index c83aced9f81..e78487f4407 100644
--- a/source3/utils/net_ads.c
+++ b/source3/utils/net_ads.c
@@ -2438,9 +2438,12 @@ static int net_ads_search(struct net_context
*c, int argc, const char **argv)
        ldap_exp = argv[0];
        attrs = (argv + 1);

+       ads->is_mine = False;
+
        rc = ads_do_search_retry(ads, ads->config.bind_path,
                               LDAP_SCOPE_SUBTREE,
                               ldap_exp, attrs, &res);
+       ads->is_mine = True;
        if (!ADS_ERR_OK(rc)) {
                d_fprintf(stderr, _("search failed: %s\n"), ads_errstr(rc));
                ads_destroy(&ads);

Thanks!