Attachment 14198 Details for Bug 13420 – patch for 4.8 cherry-picked from master

[patch] patch for 4.8 cherry-picked from master

s4-lsa-use-after-free.patch (text/plain), 5.08 KB, created by Andrew Bartlett on 2018-05-13 08:55:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2018-05-13 08:55:05 UTC

Size: 5.08 KB

patch

obsolete

>From 3359d26257878fdacd7488d3ca042ad6e918b688 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Thu, 3 May 2018 16:22:19 +1200
>Subject: [PATCH 1/2] s4-lsa: Fix use-after-free in LSA server
>
>This is a regression introduced in ab7988aa2fd1a43f576a4b73a6893c61c7ef1957.
>
>The state variable contains the data to be returned to the client
>and packed into NDR after the function returned.
>
>This memory needs to be kept (on mem_ctx as parent) until that is
>pushed and freed by the caller.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13420
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 7e091e505156381e385235ab4518b4d133a98497)
>---
> source4/rpc_server/lsa/lsa_lookup.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/source4/rpc_server/lsa/lsa_lookup.c b/source4/rpc_server/lsa/lsa_lookup.c
>index 3baff1ec11f..1b6a7d2e5a7 100644
>--- a/source4/rpc_server/lsa/lsa_lookup.c
>+++ b/source4/rpc_server/lsa/lsa_lookup.c
>@@ -805,7 +805,6 @@ NTSTATUS dcesrv_lsa_LookupSids(struct dcesrv_call_state *dce_call, TALLOC_CTX *m
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupSids_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>@@ -1284,7 +1283,6 @@ NTSTATUS dcesrv_lsa_LookupNames3(struct dcesrv_call_state *dce_call,
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupNames_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>@@ -1357,7 +1355,6 @@ NTSTATUS dcesrv_lsa_LookupNames4(struct dcesrv_call_state *dce_call, TALLOC_CTX
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupNames_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>-- 
>2.14.3
>
>
>From 43e514424cec0ede63e7dd06bfabdaa1291226c8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 11 May 2018 06:43:14 +0200
>Subject: [PATCH 2/2] s4:lsa_lookup: remove TALLOC_FREE(state) after all
> dcesrv_lsa_Lookup{Names,Sids}_base_map() calls
>
>This completes the regression fix of commit 7e091e505156381e385235ab4518b4d133a98497.
>
>There might be strings allocated on state, which are part of the
>result.
>
>The reason for the TALLOC_FREE(state) was to cleanup the possible
>irpc_handle before leaving the function. Now we call
>TALLOC_FREE(state->wb.irpc_handle) explicitly in
>dcesrv_lsa_Lookup{Names,Sids}_base_done() instead.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13420
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Sun May 13 10:27:28 CEST 2018 on sn-devel-144
>
>(cherry picked from commit 9a513304adadd79d1c63d55fcf06b67ed45d43ba)
>---
> source4/rpc_server/lsa/lsa_lookup.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
>diff --git a/source4/rpc_server/lsa/lsa_lookup.c b/source4/rpc_server/lsa/lsa_lookup.c
>index 1b6a7d2e5a7..e55539cf7b2 100644
>--- a/source4/rpc_server/lsa/lsa_lookup.c
>+++ b/source4/rpc_server/lsa/lsa_lookup.c
>@@ -533,6 +533,7 @@ static void dcesrv_lsa_LookupSids_base_done(struct tevent_req *subreq)
> 	status = dcerpc_lsa_LookupSids3_recv(subreq, state->mem_ctx,
> 					     &state->wb.result);
> 	TALLOC_FREE(subreq);
>+	TALLOC_FREE(state->wb.irpc_handle);
> 	if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
> 		DEBUG(0,(__location__ ": IRPC callback failed %s\n",
> 			 nt_errstr(status)));
>@@ -598,7 +599,6 @@ static void dcesrv_lsa_LookupSids_base_done(struct tevent_req *subreq)
>  finished:
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupSids_base_map(state);
>-	TALLOC_FREE(state);
> 
> 	status = dcesrv_reply(dce_call);
> 	if (!NT_STATUS_IS_OK(status)) {
>@@ -660,7 +660,6 @@ NTSTATUS dcesrv_lsa_LookupSids2(struct dcesrv_call_state *dce_call,
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupSids_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>@@ -734,7 +733,6 @@ NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call,
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupSids_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>@@ -1155,6 +1153,7 @@ static void dcesrv_lsa_LookupNames_base_done(struct tevent_req *subreq)
> 	status = dcerpc_lsa_LookupNames4_recv(subreq, state->mem_ctx,
> 					      &state->wb.result);
> 	TALLOC_FREE(subreq);
>+	TALLOC_FREE(state->wb.irpc_handle);
> 	if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
> 		DEBUG(0,(__location__ ": IRPC callback failed %s\n",
> 			 nt_errstr(status)));
>@@ -1220,7 +1219,6 @@ static void dcesrv_lsa_LookupNames_base_done(struct tevent_req *subreq)
>  finished:
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupNames_base_map(state);
>-	TALLOC_FREE(state);
> 
> 	status = dcesrv_reply(dce_call);
> 	if (!NT_STATUS_IS_OK(status)) {
>@@ -1433,7 +1431,6 @@ NTSTATUS dcesrv_lsa_LookupNames2(struct dcesrv_call_state *dce_call,
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupNames_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>@@ -1504,7 +1501,6 @@ NTSTATUS dcesrv_lsa_LookupNames(struct dcesrv_call_state *dce_call, TALLOC_CTX *
> 
> 	state->r.out.result = status;
> 	dcesrv_lsa_LookupNames_base_map(state);
>-	TALLOC_FREE(state);
> 	return status;
> }
> 
>-- 
>2.14.3
>

Flags:

metze: review+

Actions: View

Attachments on bug 13420: 14172 | 14174 | 14192 | 14195 | 14196 | 14198