From 3359d26257878fdacd7488d3ca042ad6e918b688 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett 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 Reviewed-by: Douglas Bagnall (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 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 Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett 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