[URGENT][PATCH] Re: Possible issue in AD DC LSA server in master

[Stefan Metzmacher]

Am 03.05.2018 um 10:15 schrieb Andrew Bartlett via samba-technical:
> On Thu, 2018-05-03 at 09:43 +0200, Stefan Metzmacher wrote:
>> Am 03.05.2018 um 06:29 schrieb Andrew Bartlett via samba-technical:
>>> On Sat, 2018-04-14 at 07:07 +1200, Andrew Bartlett via samba-technical
>>> wrote:
>>>> So, that autobuild failed with:
>>>>
>>>>> [133(711)/525 at 10m56s] samba3.rpc.lsa.lookupsids(ad_dc)
>>>>> smbtorture 4.9.0pre1-DEVELOPERBUILD
>>>>> Using seed 1523610756
>>>>> UNEXPECTED(failure): samba3.rpc.lsa.lookupsids.lsa.LookupSidsReply(ad_dc)
>>>>> REASON: Exception: Exception: ../source4/torture/rpc/lsa_lookup.c:400: names.names[0].name.string was , expected S-1-5-21-1111111111-2222222222-3333333333-512: unexpected names[0].string
>>>>>
>>>>> FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
>>>>
>>>> And I mentioned before that I got one LSA failure on the branch up to:
>>>>
>>>> commit cb607346d3c7c662343b0eae69e43eaa6358c188
>>>> Author: Gary Lockyer <gary at catalyst.net.nz>
>>>> Date:   Tue Mar 13 16:43:54 2018 +1300
>>>>
>>>>     ldb-samba: require pid match for cached ldb
>>>>     
>>>>     Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
>>>>     Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>>>>
>>>>> Testing OpenPolicy2
>>>>> UNEXPECTED(failure): samba4.rpc.altercontext on ncalrpc with seal,padcheck.altercontext(ad_dc_ntvfs:local)
>>>>> REASON: Exception: Exception: ../source4/torture/rpc/lsa.c:188: status was NT_STATUS_CONNECTION_RESET, expected NT_STATUS_CONNECTION_DISCONNECTED: OpenPolicy2 failed
>>>>
>>>> (but wrote it off as I also got about 10 successes on branches with
>>>> that series in it). 
>>>>
>>>> This is fishy, as Joe yesterday got this in travis CI on master:
>>>>
>>>>> Testing LookupSids
>>>>> ndr_pull_error(1): Bad array size - got 0 expected 8
>>>>>
>>>>> UNEXPECTED(failure): samba3.rpc.lsa.privileges.lsa.Privileges(ad_dc)
>>>>> REASON: Exception: Exception: ../source4/torture/rpc/lsa.c:774: dcerpc_lsa_LookupSids_r(b, tctx, &r) was NT_STATUS_ARRAY_BOUNDS_EXCEEDED, expected NT_STATUS_OK: LookupSids failed
>>>>>
>>>>> FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
>>>>
>>>> If anybody has any insights or suggestions please don't hesitate to
>>>> investigate.
>>>
>>> It turned out to be an unrelated use-after-free in the LSA server after
>>> the trusts changes recently. 
>>>
>>> The issue was found fairly easily with address-sanitizer and is fixed
>>> in the attached.  This needs to be in 4.8.2 the regression was shipped
>>> with 4.8.0.
>>
>> Do you have more detailed information on what memory is used after free?
> 
> It shows up pretty fast under address sanitizer with the other patches
> posted today.  It seemed so ovbious that I didn't go much further in
> making notes, but here is the backtrace.
> 
> https://attachments.samba.org/attachment.cgi?id=14174
> 
>> I'd prefer to do the correct talloc_move() calls in order to get a sane
>> memory tree instead of being lazy.
> 
> Have a look at:
> dcesrv_lsa_LookupSids()
> 	state->r.out.names = talloc_zero(state, struct
> lsa_TransNameArray2);

I guess this would also fix it?

metze

-------------- next part --------------
From 85d1d78a10581acf9a7b8942a3c294b37edd6adb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 3 May 2018 14:00:10 +0200
Subject: [PATCH] fix dcesrv_lsa_LookupSids_base_map

---
 source4/rpc_server/lsa/lsa_lookup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source4/rpc_server/lsa/lsa_lookup.c b/source4/rpc_server/lsa/lsa_lookup.c
index f7d367e..3da7296 100644
--- a/source4/rpc_server/lsa/lsa_lookup.c
+++ b/source4/rpc_server/lsa/lsa_lookup.c
@@ -515,6 +515,8 @@ static void dcesrv_lsa_LookupSids_base_map(
 			n->sid_type = n2->sid_type;
 			n->name = n2->name;
 			n->sid_index = n2->sid_index;
+
+			talloc_steal(r->out.names->names, n->name.string);
 		}
 		r->out.names->count = state->r.out.names->count;
 		return;
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180503/cad37228/signature.sig>