From 0e5910c6abe8e402d97f4bb64b07b5a47850ea9f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 24 Sep 2013 05:03:40 +0200 Subject: [PATCH 01/15] CVE-2013-4408:librpc: check for invalid frag_len within dcerpc_read_ncacn_packet_done() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- librpc/rpc/dcerpc_util.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index a405ca8..632d70d 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -292,6 +292,11 @@ static void dcerpc_read_ncacn_packet_done(struct tevent_req *subreq) return; } + if (state->pkt->frag_length != state->buffer.length) { + tevent_req_nterror(req, NT_STATUS_RPC_PROTOCOL_ERROR); + return; + } + tevent_req_done(req); } -- 1.7.9.5 From 0fac736696ddeeaae199b639657b2990febfc4a3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 24 Sep 2013 05:03:40 +0200 Subject: [PATCH 02/15] CVE-2013-4408:librpc: check for invalid frag_len within dcerpc_read_ncacn_packet_next_vector() We should do this explicit instead of relying on tstream_readv_pdu_ask_for_next_vector() to catch the overflow. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- librpc/rpc/dcerpc_util.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index 632d70d..b8bf64d 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -223,6 +223,15 @@ static int dcerpc_read_ncacn_packet_next_vector(struct tstream_context *stream, ofs = state->buffer.length; + if (frag_len < ofs) { + /* + * something is wrong, let the caller deal with it + */ + *_vector = NULL; + *_count = 0; + return 0; + } + state->buffer.data = talloc_realloc(state, state->buffer.data, uint8_t, frag_len); -- 1.7.9.5 From 2fb08e5a818a18fe1d6e7b0a94b35151f61e00cb Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 25 Sep 2013 23:25:12 +0200 Subject: [PATCH 03/15] CVE-2013-4408:s3:rpc_client: check for invalid frag_len in dcerpc_pull_ncacn_packet() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source3/librpc/rpc/dcerpc_helpers.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/librpc/rpc/dcerpc_helpers.c b/source3/librpc/rpc/dcerpc_helpers.c index 469e308..24f2f52 100644 --- a/source3/librpc/rpc/dcerpc_helpers.c +++ b/source3/librpc/rpc/dcerpc_helpers.c @@ -129,6 +129,10 @@ NTSTATUS dcerpc_pull_ncacn_packet(TALLOC_CTX *mem_ctx, NDR_PRINT_DEBUG(ncacn_packet, r); } + if (r->frag_length != blob->length) { + return NT_STATUS_RPC_PROTOCOL_ERROR; + } + return NT_STATUS_OK; } -- 1.7.9.5 From 7d178e03fcee330e389178cd95914834f139770b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 25 Sep 2013 23:25:12 +0200 Subject: [PATCH 04/15] CVE-2013-4408:s3:rpc_client: verify frag_len at least contains the header size Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source3/rpc_client/cli_pipe.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source3/rpc_client/cli_pipe.c b/source3/rpc_client/cli_pipe.c index 4e5fd71..57dcddc 100644 --- a/source3/rpc_client/cli_pipe.c +++ b/source3/rpc_client/cli_pipe.c @@ -281,6 +281,10 @@ static struct tevent_req *get_complete_frag_send(TALLOC_CTX *mem_ctx, } state->frag_len = dcerpc_get_frag_length(pdu); + if (state->frag_len < RPC_HEADER_LEN) { + tevent_req_nterror(req, NT_STATUS_RPC_PROTOCOL_ERROR); + return tevent_req_post(req, ev); + } /* * Ensure we have frag_len bytes of data. @@ -329,6 +333,10 @@ static void get_complete_frag_got_header(struct tevent_req *subreq) } state->frag_len = dcerpc_get_frag_length(state->pdu); + if (state->frag_len < RPC_HEADER_LEN) { + tevent_req_nterror(req, NT_STATUS_RPC_PROTOCOL_ERROR); + return; + } if (!data_blob_realloc(NULL, state->pdu, state->frag_len)) { tevent_req_nterror(req, NT_STATUS_NO_MEMORY); -- 1.7.9.5 From 53ec8394dd36dcbcade30dd04407cec7d47e57a1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 25 Sep 2013 23:25:12 +0200 Subject: [PATCH 05/15] CVE-2013-4408:s4:dcerpc: check for invalid frag_len in ncacn_pull() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source4/librpc/rpc/dcerpc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c index cc72866..742d710 100644 --- a/source4/librpc/rpc/dcerpc.c +++ b/source4/librpc/rpc/dcerpc.c @@ -658,6 +658,10 @@ static NTSTATUS ncacn_pull(struct dcecli_connection *c, DATA_BLOB *blob, TALLOC_ return ndr_map_error2ntstatus(ndr_err); } + if (pkt->frag_length != blob->length) { + return NT_STATUS_RPC_PROTOCOL_ERROR; + } + return NT_STATUS_OK; } -- 1.7.9.5 From 132e0db8e0d9c6f89ac6e478146cb28ce80ae574 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 25 Sep 2013 23:25:12 +0200 Subject: [PATCH 06/15] CVE-2013-4408:s4:dcerpc_smb: check for invalid frag_len in send_read_request_continue() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source4/librpc/rpc/dcerpc_smb.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source4/librpc/rpc/dcerpc_smb.c b/source4/librpc/rpc/dcerpc_smb.c index 395e067..61cf791 100644 --- a/source4/librpc/rpc/dcerpc_smb.c +++ b/source4/librpc/rpc/dcerpc_smb.c @@ -160,6 +160,12 @@ static NTSTATUS send_read_request_continue(struct dcecli_connection *c, DATA_BLO } else { uint32_t frag_length = blob->length>=16? dcerpc_get_frag_length(blob):0x2000; + + if (frag_length < state->data.length) { + talloc_free(state); + return NT_STATUS_RPC_PROTOCOL_ERROR; + } + state->received = blob->length; state->data = data_blob_talloc(state, NULL, frag_length); if (!state->data.data) { -- 1.7.9.5 From 8b7497008f06a1d7c50129b7a803f41b7190dd99 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 25 Sep 2013 23:25:12 +0200 Subject: [PATCH 07/15] CVE-2013-4408:s4:dcerpc_smb2: check for invalid frag_len in send_read_request_continue() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source4/librpc/rpc/dcerpc_smb2.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source4/librpc/rpc/dcerpc_smb2.c b/source4/librpc/rpc/dcerpc_smb2.c index 50aed8c..4539ead 100644 --- a/source4/librpc/rpc/dcerpc_smb2.c +++ b/source4/librpc/rpc/dcerpc_smb2.c @@ -170,6 +170,12 @@ static NTSTATUS send_read_request_continue(struct dcecli_connection *c, DATA_BLO if (state->data.length >= 16) { uint16_t frag_length = dcerpc_get_frag_length(&state->data); + + if (frag_length < state->data.length) { + talloc_free(state); + return NT_STATUS_RPC_PROTOCOL_ERROR; + } + io.in.length = frag_length - state->data.length; } else { io.in.length = 0x2000; -- 1.7.9.5 From 9af31b5c80821749b3153fd7c85eecc9db56a6c7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 24 Sep 2013 05:03:40 +0200 Subject: [PATCH 08/15] CVE-2013-4408:s4:dcerpc_sock: check for invalid frag_len within sock_complete_packet() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source4/librpc/rpc/dcerpc_sock.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source4/librpc/rpc/dcerpc_sock.c b/source4/librpc/rpc/dcerpc_sock.c index f0451ac..9a596da 100644 --- a/source4/librpc/rpc/dcerpc_sock.c +++ b/source4/librpc/rpc/dcerpc_sock.c @@ -102,6 +102,12 @@ static NTSTATUS sock_complete_packet(void *private_data, DATA_BLOB blob, size_t return STATUS_MORE_ENTRIES; } *size = dcerpc_get_frag_length(&blob); + if (*size < blob.length) { + /* + * something is wrong, let the caller deal with it + */ + *size = blob.length; + } if (*size > blob.length) { return STATUS_MORE_ENTRIES; } -- 1.7.9.5 From 80ad1bfec28825afa7f9b8d0d50b7911098bf3b5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Oct 2013 14:17:49 +0200 Subject: [PATCH 09/15] CVE-2013-4408:async_sock: add some overflow detection to read_packet_handler() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- lib/async_req/async_sock.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c index bb8518f..03b0b33 100644 --- a/lib/async_req/async_sock.c +++ b/lib/async_req/async_sock.c @@ -635,6 +635,11 @@ static void read_packet_handler(struct tevent_context *ev, return; } + if (total + more < total) { + tevent_req_error(req, EMSGSIZE); + return; + } + tmp = talloc_realloc(state, state->buf, uint8_t, total+more); if (tevent_req_nomem(tmp, req)) { return; -- 1.7.9.5 From 852e4419041f997a476e778da8e1052957495f42 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Oct 2013 14:17:49 +0200 Subject: [PATCH 10/15] CVE-2013-4408:s3:util_tsock: add some overflow detection to tstream_read_packet_done() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source3/lib/util_tsock.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/lib/util_tsock.c b/source3/lib/util_tsock.c index 35a97f5..03380ef 100644 --- a/source3/lib/util_tsock.c +++ b/source3/lib/util_tsock.c @@ -110,6 +110,11 @@ static void tstream_read_packet_done(struct tevent_req *subreq) return; } + if (total + more < total) { + tevent_req_error(req, EMSGSIZE); + return; + } + tmp = talloc_realloc(state, state->buf, uint8_t, total+more); if (tevent_req_nomem(tmp, req)) { return; -- 1.7.9.5 From 5f9e16277afe63ccd7b7fc19d0d380d6c9b7618d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Oct 2013 14:17:49 +0200 Subject: [PATCH 11/15] CVE-2013-4408:libcli/util: add some size verification to tstream_read_pdu_blob_done() Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- libcli/util/tstream.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libcli/util/tstream.c b/libcli/util/tstream.c index b287597..ff7f864 100644 --- a/libcli/util/tstream.c +++ b/libcli/util/tstream.c @@ -129,6 +129,11 @@ static void tstream_read_pdu_blob_done(struct tevent_req *subreq) return; } + if (new_buf_size <= old_buf_size) { + tevent_req_nterror(req, NT_STATUS_INVALID_BUFFER_SIZE); + return; + } + buf = talloc_realloc(state, state->pdu_blob.data, uint8_t, new_buf_size); if (tevent_req_nomem(buf, req)) { return; -- 1.7.9.5 From 1f14621cdbf960fff3e838afa5a345090cbb4cf9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 22 Oct 2013 15:34:12 -0700 Subject: [PATCH 12/15] CVE-2013-4408:s3:Ensure we always check call_id when validating an RPC reply. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Jeremy Allison Reviewed-by: Stefan Metzmacher --- librpc/idl/dcerpc.idl | 1 + librpc/rpc/dcerpc_util.c | 9 +++++++++ librpc/rpc/rpc_common.h | 1 + source3/rpc_client/cli_pipe.c | 34 ++++++++++++++++++++++++++++------ 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/librpc/idl/dcerpc.idl b/librpc/idl/dcerpc.idl index 86f22a4..8949836 100644 --- a/librpc/idl/dcerpc.idl +++ b/librpc/idl/dcerpc.idl @@ -467,6 +467,7 @@ interface dcerpc const uint8 DCERPC_DREP_OFFSET = 4; const uint8 DCERPC_FRAG_LEN_OFFSET = 8; const uint8 DCERPC_AUTH_LEN_OFFSET = 10; + const uint8 DCERPC_CALL_ID_OFFSET = 12; /* little-endian flag */ const uint8 DCERPC_DREP_LE = 0x10; diff --git a/librpc/rpc/dcerpc_util.c b/librpc/rpc/dcerpc_util.c index b8bf64d..cb21312 100644 --- a/librpc/rpc/dcerpc_util.c +++ b/librpc/rpc/dcerpc_util.c @@ -48,6 +48,15 @@ uint16_t dcerpc_get_frag_length(const DATA_BLOB *blob) } } +uint32_t dcerpc_get_call_id(const DATA_BLOB *blob) +{ + if (CVAL(blob->data,DCERPC_DREP_OFFSET) & DCERPC_DREP_LE) { + return IVAL(blob->data, DCERPC_CALL_ID_OFFSET); + } else { + return RIVAL(blob->data, DCERPC_CALL_ID_OFFSET); + } +} + void dcerpc_set_auth_length(DATA_BLOB *blob, uint16_t v) { if (CVAL(blob->data,DCERPC_DREP_OFFSET) & DCERPC_DREP_LE) { diff --git a/librpc/rpc/rpc_common.h b/librpc/rpc/rpc_common.h index 44c3cfd..924645d 100644 --- a/librpc/rpc/rpc_common.h +++ b/librpc/rpc/rpc_common.h @@ -135,6 +135,7 @@ enum dcerpc_transport_t dcerpc_transport_by_tower(const struct epm_tower *tower) void dcerpc_set_frag_length(DATA_BLOB *blob, uint16_t v); uint16_t dcerpc_get_frag_length(const DATA_BLOB *blob); +uint32_t dcerpc_get_call_id(const DATA_BLOB *blob); void dcerpc_set_auth_length(DATA_BLOB *blob, uint16_t v); uint8_t dcerpc_get_endian_flag(DATA_BLOB *blob); diff --git a/source3/rpc_client/cli_pipe.c b/source3/rpc_client/cli_pipe.c index 57dcddc..9636479 100644 --- a/source3/rpc_client/cli_pipe.c +++ b/source3/rpc_client/cli_pipe.c @@ -235,6 +235,7 @@ struct get_complete_frag_state { struct event_context *ev; struct rpc_pipe_client *cli; uint16_t frag_len; + uint32_t call_id; DATA_BLOB *pdu; }; @@ -244,6 +245,7 @@ static void get_complete_frag_got_rest(struct tevent_req *subreq); static struct tevent_req *get_complete_frag_send(TALLOC_CTX *mem_ctx, struct event_context *ev, struct rpc_pipe_client *cli, + uint32_t call_id, DATA_BLOB *pdu) { struct tevent_req *req, *subreq; @@ -259,6 +261,7 @@ static struct tevent_req *get_complete_frag_send(TALLOC_CTX *mem_ctx, state->ev = ev; state->cli = cli; state->frag_len = RPC_HEADER_LEN; + state->call_id = call_id; state->pdu = pdu; received = pdu->length; @@ -286,6 +289,11 @@ static struct tevent_req *get_complete_frag_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + if (state->call_id != dcerpc_get_call_id(pdu)) { + tevent_req_nterror(req, NT_STATUS_RPC_PROTOCOL_ERROR); + return tevent_req_post(req, ev); + } + /* * Ensure we have frag_len bytes of data. */ @@ -338,6 +346,11 @@ static void get_complete_frag_got_header(struct tevent_req *subreq) return; } + if (state->call_id != dcerpc_get_call_id(state->pdu)) { + tevent_req_nterror(req, NT_STATUS_RPC_PROTOCOL_ERROR); + return; + } + if (!data_blob_realloc(NULL, state->pdu, state->frag_len)) { tevent_req_nterror(req, NT_STATUS_NO_MEMORY); return; @@ -698,6 +711,7 @@ struct rpc_api_pipe_state { struct event_context *ev; struct rpc_pipe_client *cli; uint8_t expected_pkt_type; + uint32_t call_id; DATA_BLOB incoming_frag; struct ncacn_packet *pkt; @@ -716,7 +730,8 @@ static struct tevent_req *rpc_api_pipe_send(TALLOC_CTX *mem_ctx, struct event_context *ev, struct rpc_pipe_client *cli, DATA_BLOB *data, /* Outgoing PDU */ - uint8_t expected_pkt_type) + uint8_t expected_pkt_type, + uint32_t call_id) { struct tevent_req *req, *subreq; struct rpc_api_pipe_state *state; @@ -730,6 +745,7 @@ static struct tevent_req *rpc_api_pipe_send(TALLOC_CTX *mem_ctx, state->ev = ev; state->cli = cli; state->expected_pkt_type = expected_pkt_type; + state->call_id = call_id; state->incoming_frag = data_blob_null; state->reply_pdu = data_blob_null; state->reply_pdu_offset = 0; @@ -829,6 +845,7 @@ static void rpc_api_pipe_trans_done(struct tevent_req *subreq) /* Ensure we have enough data for a pdu. */ subreq = get_complete_frag_send(state, state->ev, state->cli, + state->call_id, &state->incoming_frag); if (tevent_req_nomem(subreq, req)) { return; @@ -948,6 +965,7 @@ static void rpc_api_pipe_got_pdu(struct tevent_req *subreq) } subreq = get_complete_frag_send(state, state->ev, state->cli, + state->call_id, &state->incoming_frag); if (tevent_req_nomem(subreq, req)) { return; @@ -1300,7 +1318,8 @@ struct tevent_req *rpc_api_pipe_req_send(TALLOC_CTX *mem_ctx, if (is_last_frag) { subreq = rpc_api_pipe_send(state, ev, state->cli, &state->rpc_out, - DCERPC_PKT_RESPONSE); + DCERPC_PKT_RESPONSE, + state->call_id); if (subreq == NULL) { goto fail; } @@ -1436,7 +1455,8 @@ static void rpc_api_pipe_req_write_done(struct tevent_req *subreq) if (is_last_frag) { subreq = rpc_api_pipe_send(state, state->ev, state->cli, &state->rpc_out, - DCERPC_PKT_RESPONSE); + DCERPC_PKT_RESPONSE, + state->call_id); if (tevent_req_nomem(subreq, req)) { return; } @@ -1675,7 +1695,7 @@ struct tevent_req *rpc_pipe_bind_send(TALLOC_CTX *mem_ctx, } subreq = rpc_api_pipe_send(state, ev, cli, &state->rpc_out, - DCERPC_PKT_BIND_ACK); + DCERPC_PKT_BIND_ACK, state->rpc_call_id); if (subreq == NULL) { goto fail; } @@ -1873,7 +1893,8 @@ static NTSTATUS rpc_bind_next_send(struct tevent_req *req, } subreq = rpc_api_pipe_send(state, state->ev, state->cli, - &state->rpc_out, DCERPC_PKT_ALTER_RESP); + &state->rpc_out, DCERPC_PKT_ALTER_RESP, + state->rpc_call_id); if (subreq == NULL) { return NT_STATUS_NO_MEMORY; } @@ -1905,7 +1926,8 @@ static NTSTATUS rpc_bind_finish_send(struct tevent_req *req, } subreq = rpc_api_pipe_send(state, state->ev, state->cli, - &state->rpc_out, DCERPC_PKT_AUTH3); + &state->rpc_out, DCERPC_PKT_AUTH3, + state->rpc_call_id); if (subreq == NULL) { return NT_STATUS_NO_MEMORY; } -- 1.7.9.5 From 6a2899a34825aec871697064e8e25e2177972b49 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 19 Nov 2013 13:53:32 -0800 Subject: [PATCH 13/15] CVE-2013-4408:s3:Ensure LookupSids replies arrays are range checked. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Signed-off-by: Jeremy Allison --- nsswitch/libwbclient/wbc_sid.c | 7 +++++++ nsswitch/wbinfo.c | 23 ++++++++++++++++++++--- source3/rpc_client/cli_lsarpc.c | 17 ++++++++++++++++- source3/rpcclient/cmd_lsarpc.c | 7 +++++-- source3/winbindd/wb_lookupsids.c | 3 +++ source3/winbindd/winbindd_rpc.c | 32 ++++++++++++++++++++++++++++++++ source4/libcli/util/clilsa.c | 16 +++++++++++++++- source4/winbind/wb_async_helpers.c | 13 ++++++++++++- 8 files changed, 110 insertions(+), 8 deletions(-) diff --git a/nsswitch/libwbclient/wbc_sid.c b/nsswitch/libwbclient/wbc_sid.c index 6df8a3c..35319c5 100644 --- a/nsswitch/libwbclient/wbc_sid.c +++ b/nsswitch/libwbclient/wbc_sid.c @@ -421,6 +421,13 @@ wbcErr wbcLookupSids(const struct wbcDomainSid *sids, int num_sids, for (i=0; i= num_domains) { + goto wbc_err_invalid; + } + if (*q != ' ') { goto wbc_err_invalid; } diff --git a/nsswitch/wbinfo.c b/nsswitch/wbinfo.c index 9d25f59..8b822d7 100644 --- a/nsswitch/wbinfo.c +++ b/nsswitch/wbinfo.c @@ -1380,11 +1380,28 @@ static bool wbinfo_lookup_sids(const char *arg) } for (i=0; i %s\\%s %d\n", sidstr, - domains[names[i].domain_index].short_name, - names[i].name, names[i].type); + if (names[i].domain_index >= num_domains) { + domain = ""; + } else if (names[i].domain_index < 0) { + domain = ""; + } else { + domain = domains[names[i].domain_index].short_name; + } + + if (names[i].type == WBC_SID_NAME_DOMAIN) { + d_printf("%s -> %s %d\n", sidstr, + domain, + names[i].type); + } else { + d_printf("%s -> %s%c%s %d\n", sidstr, + domain, + winbind_separator(), + names[i].name, names[i].type); + } } return true; } diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c index 330774d..e7e8236 100644 --- a/source3/rpc_client/cli_lsarpc.c +++ b/source3/rpc_client/cli_lsarpc.c @@ -279,11 +279,26 @@ static NTSTATUS dcerpc_lsa_lookup_sids_noalloc(struct dcerpc_binding_handle *h, for (i = 0; i < num_sids; i++) { const char *name, *dom_name; - uint32_t dom_idx = lsa_names.names[i].sid_index; + uint32_t dom_idx; + + if (i >= lsa_names.count) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + return status; + } + + dom_idx = lsa_names.names[i].sid_index; /* Translate optimised name through domain index array */ if (dom_idx != 0xffffffff) { + if (ref_domains == NULL) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + return status; + } + if (dom_idx >= ref_domains->count) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + return status; + } dom_name = ref_domains->domains[dom_idx].name.string; name = lsa_names.names[i].name.string; diff --git a/source3/rpcclient/cmd_lsarpc.c b/source3/rpcclient/cmd_lsarpc.c index 3aaef5c..9893a7c 100644 --- a/source3/rpcclient/cmd_lsarpc.c +++ b/source3/rpcclient/cmd_lsarpc.c @@ -450,7 +450,7 @@ static NTSTATUS cmd_lsa_lookup_sids3(struct rpc_pipe_client *cli, NTSTATUS status = NT_STATUS_UNSUCCESSFUL, result; int i; struct lsa_SidArray sids; - struct lsa_RefDomainList *domains; + struct lsa_RefDomainList *domains = NULL; struct lsa_TransNameArray2 names; uint32_t count = 0; struct dcerpc_binding_handle *b = cli->binding_handle; @@ -506,9 +506,12 @@ static NTSTATUS cmd_lsa_lookup_sids3(struct rpc_pipe_client *cli, /* Print results */ - for (i = 0; i < count; i++) { + for (i = 0; i < names.count; i++) { fstring sid_str; + if (i >= sids.num_sids) { + break; + } sid_to_fstring(sid_str, sids.sids[i].sid); printf("%s %s (%d)\n", sid_str, names.names[i].name.string, diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c index 2fd735d..32f646c 100644 --- a/source3/winbindd/wb_lookupsids.c +++ b/source3/winbindd/wb_lookupsids.c @@ -402,6 +402,9 @@ static bool wb_lookupsids_move_name(struct lsa_RefDomainList *src_domains, uint32_t src_domain_index, dst_domain_index; src_domain_index = src_name->sid_index; + if (src_domain_index >= src_domains->count) { + return false; + } src_domain = &src_domains->domains[src_domain_index]; if (!wb_lookupsids_find_dom_idx( diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c index 9a95e57..ad4ce44 100644 --- a/source3/winbindd/winbindd_rpc.c +++ b/source3/winbindd/winbindd_rpc.c @@ -1060,6 +1060,10 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx, if (NT_STATUS_IS_ERR(result)) { return result; } + if (sids->num_sids != lsa_names2.count) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + names = TALLOC_ZERO_P(mem_ctx, struct lsa_TransNameArray); if (names == NULL) { return NT_STATUS_NO_MEMORY; @@ -1075,6 +1079,16 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx, names->names[i].name.string = talloc_move( names->names, &lsa_names2.names[i].name.string); names->names[i].sid_index = lsa_names2.names[i].sid_index; + + if (names->names[i].sid_index == UINT32_MAX) { + continue; + } + if ((*pdomains) == NULL) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (names->names[i].sid_index >= (*pdomains)->count) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } } *pnames = names; return result; @@ -1090,6 +1104,7 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx, struct rpc_pipe_client *cli = NULL; struct policy_handle lsa_policy; uint32_t count; + uint32_t i; NTSTATUS status, result; status = cm_connect_lsat(domain, mem_ctx, &cli, &lsa_policy); @@ -1116,6 +1131,23 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx, if (NT_STATUS_IS_ERR(result)) { return result; } + + if (sids->num_sids != names->count) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + + for (i=0; i < names->count; i++) { + if (names->names[i].sid_index == UINT32_MAX) { + continue; + } + if ((*pdomains) == NULL) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (names->names[i].sid_index >= (*pdomains)->count) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + } + *pnames = names; return result; } diff --git a/source4/libcli/util/clilsa.c b/source4/libcli/util/clilsa.c index 4cfdf93..d705abd 100644 --- a/source4/libcli/util/clilsa.c +++ b/source4/libcli/util/clilsa.c @@ -254,7 +254,21 @@ NTSTATUS smblsa_lookup_sid(struct smbcli_state *cli, } if (names.count != 1) { talloc_free(mem_ctx2); - return NT_STATUS_UNSUCCESSFUL; + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (domains == NULL) { + talloc_free(mem_ctx2); + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (domains->count != 1) { + talloc_free(mem_ctx2); + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (names.names[0].sid_index != UINT32_MAX && + names.names[0].sid_index >= domains->count) + { + talloc_free(mem_ctx2); + return NT_STATUS_INVALID_NETWORK_RESPONSE; } (*name) = talloc_asprintf(mem_ctx, "%s\\%s", diff --git a/source4/winbind/wb_async_helpers.c b/source4/winbind/wb_async_helpers.c index 5d530ca..01ff4a4 100644 --- a/source4/winbind/wb_async_helpers.c +++ b/source4/winbind/wb_async_helpers.c @@ -120,6 +120,12 @@ static void lsa_lookupsids_recv_names(struct tevent_req *subreq) return; } + if (state->names.count != state->num_sids) { + composite_error(state->ctx, + NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } + state->result = talloc_array(state, struct wb_sid_object *, state->num_sids); if (composite_nomem(state->result, state->ctx)) return; @@ -140,9 +146,14 @@ static void lsa_lookupsids_recv_names(struct tevent_req *subreq) continue; } + if (domains == NULL) { + composite_error(state->ctx, + NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } if (name->sid_index >= domains->count) { composite_error(state->ctx, - NT_STATUS_INVALID_PARAMETER); + NT_STATUS_INVALID_NETWORK_RESPONSE); return; } -- 1.7.9.5 From 257d00fe3817e0615bab2989247a97b998069423 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 19 Nov 2013 14:04:19 -0800 Subject: [PATCH 14/15] CVE-2013-4408:s3:Ensure LookupNames replies arrays are range checked. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Stefan Metzmacher Signed-off-by: Jeremy Allison --- source3/lib/netapi/group.c | 82 +++++++++++++++++++++++++++ source3/lib/netapi/localgroup.c | 8 ++- source3/lib/netapi/user.c | 56 ++++++++++++++++++ source3/libnet/libnet_join.c | 16 ++++++ source3/rpc_client/cli_lsarpc.c | 18 ++++++ source3/rpc_server/netlogon/srv_netlog_nt.c | 2 +- source3/rpcclient/cmd_lsarpc.c | 6 +- source3/rpcclient/cmd_samr.c | 58 ++++++++++++++++++- source3/smbd/lanman.c | 8 +++ source3/utils/net_rpc.c | 40 ++++++++++++- source3/utils/net_rpc_join.c | 9 +++ source3/winbindd/winbindd_rpc.c | 14 +---- source4/libcli/util/clilsa.c | 6 +- source4/libnet/groupinfo.c | 10 +++- source4/libnet/groupman.c | 10 ++-- source4/libnet/libnet_join.c | 12 +++- source4/libnet/libnet_lookup.c | 5 ++ source4/libnet/libnet_passwd.c | 10 +++- source4/libnet/userinfo.c | 9 ++- source4/libnet/userman.c | 24 ++++---- source4/winbind/wb_async_helpers.c | 13 ++++- 21 files changed, 370 insertions(+), 46 deletions(-) diff --git a/source3/lib/netapi/group.c b/source3/lib/netapi/group.c index 4295d9f..360640f 100644 --- a/source3/lib/netapi/group.c +++ b/source3/lib/netapi/group.c @@ -309,6 +309,15 @@ WERROR NetGroupDel_r(struct libnetapi_ctx *ctx, goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.ids[0] != SID_NAME_DOM_GRP) { werr = WERR_INVALID_DATATYPE; goto done; @@ -511,6 +520,14 @@ WERROR NetGroupSetInfo_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } if (types.ids[0] != SID_NAME_DOM_GRP) { werr = WERR_INVALID_DATATYPE; @@ -781,6 +798,14 @@ WERROR NetGroupGetInfo_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } if (types.ids[0] != SID_NAME_DOM_GRP) { werr = WERR_INVALID_DATATYPE; @@ -921,6 +946,14 @@ WERROR NetGroupAddUser_r(struct libnetapi_ctx *ctx, werr = WERR_GROUPNOTFOUND; goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } if (types.ids[0] != SID_NAME_DOM_GRP) { werr = WERR_GROUPNOTFOUND; @@ -959,6 +992,14 @@ WERROR NetGroupAddUser_r(struct libnetapi_ctx *ctx, werr = WERR_USER_NOT_FOUND; goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } if (types.ids[0] != SID_NAME_USER) { werr = WERR_USER_NOT_FOUND; @@ -1065,6 +1106,14 @@ WERROR NetGroupDelUser_r(struct libnetapi_ctx *ctx, werr = WERR_GROUPNOTFOUND; goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } if (types.ids[0] != SID_NAME_DOM_GRP) { werr = WERR_GROUPNOTFOUND; @@ -1104,6 +1153,14 @@ WERROR NetGroupDelUser_r(struct libnetapi_ctx *ctx, werr = WERR_USER_NOT_FOUND; goto done; } + if (rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } if (types.ids[0] != SID_NAME_USER) { werr = WERR_USER_NOT_FOUND; @@ -1514,6 +1571,14 @@ WERROR NetGroupGetUsers_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (group_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenGroup(b, talloc_tos(), &domain_handle, @@ -1689,6 +1754,14 @@ WERROR NetGroupSetUsers_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (group_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (group_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenGroup(b, talloc_tos(), &domain_handle, @@ -1767,6 +1840,15 @@ WERROR NetGroupSetUsers_r(struct libnetapi_ctx *ctx, goto done; } + if (r->in.num_entries != user_rids.count) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (r->in.num_entries != name_types.count) { + werr = WERR_BAD_NET_RESP; + goto done; + } + member_rids = user_rids.ids; status = dcerpc_samr_QueryGroupMember(b, talloc_tos(), diff --git a/source3/lib/netapi/localgroup.c b/source3/lib/netapi/localgroup.c index 49ba74e..d9c3c8e 100644 --- a/source3/lib/netapi/localgroup.c +++ b/source3/lib/netapi/localgroup.c @@ -58,6 +58,12 @@ static NTSTATUS libnetapi_samr_lookup_and_open_alias(TALLOC_CTX *mem_ctx, if (!NT_STATUS_IS_OK(result)) { return result; } + if (user_rids.count != 1) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (name_types.count != 1) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } switch (name_types.ids[0]) { case SID_NAME_ALIAS: @@ -1041,7 +1047,7 @@ static NTSTATUS libnetapi_lsa_lookup_names3(TALLOC_CTX *mem_ctx, NT_STATUS_NOT_OK_RETURN(result); if (count != 1 || sids.count != 1) { - return NT_STATUS_NONE_MAPPED; + return NT_STATUS_INVALID_NETWORK_RESPONSE; } sid_copy(sid, sids.sids[0].sid); diff --git a/source3/lib/netapi/user.c b/source3/lib/netapi/user.c index 653ece1..d16d226 100644 --- a/source3/lib/netapi/user.c +++ b/source3/lib/netapi/user.c @@ -604,6 +604,14 @@ WERROR NetUserDel_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (user_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenUser(b, talloc_tos(), &domain_handle, @@ -1803,6 +1811,14 @@ WERROR NetUserGetInfo_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (user_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = libnetapi_samr_lookup_user_map_USER_INFO(ctx, pipe_cli, domain_sid, @@ -1967,6 +1983,14 @@ WERROR NetUserSetInfo_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (user_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenUser(b, talloc_tos(), &domain_handle, @@ -3026,6 +3050,14 @@ WERROR NetUserGetGroups_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (user_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenUser(b, talloc_tos(), &domain_handle, @@ -3201,6 +3233,14 @@ WERROR NetUserSetGroups_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (user_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenUser(b, talloc_tos(), &domain_handle, @@ -3261,6 +3301,14 @@ WERROR NetUserSetGroups_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (group_rids.count != r->in.num_entries) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != r->in.num_entries) { + werr = WERR_BAD_NET_RESP; + goto done; + } member_rids = group_rids.ids; num_member_rids = group_rids.count; @@ -3539,6 +3587,14 @@ WERROR NetUserGetLocalGroups_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (user_rids.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (name_types.count != 1) { + werr = WERR_BAD_NET_RESP; + goto done; + } status = dcerpc_samr_OpenUser(b, talloc_tos(), &domain_handle, diff --git a/source3/libnet/libnet_join.c b/source3/libnet/libnet_join.c index e84682d..d665d72 100644 --- a/source3/libnet/libnet_join.c +++ b/source3/libnet/libnet_join.c @@ -997,6 +997,14 @@ static NTSTATUS libnet_join_joindomain_rpc(TALLOC_CTX *mem_ctx, status = result; goto done; } + if (user_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } if (name_types.ids[0] != SID_NAME_USER) { DEBUG(0,("%s is not a user account (type=%d)\n", @@ -1376,6 +1384,14 @@ static NTSTATUS libnet_join_unjoindomain_rpc(TALLOC_CTX *mem_ctx, status = result; goto done; } + if (user_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } if (name_types.ids[0] != SID_NAME_USER) { DEBUG(0, ("%s is not a user account (type=%d)\n", acct_name, diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c index e7e8236..dd1f692 100644 --- a/source3/rpc_client/cli_lsarpc.c +++ b/source3/rpc_client/cli_lsarpc.c @@ -662,9 +662,19 @@ NTSTATUS dcerpc_lsa_lookup_names_generic(struct dcerpc_binding_handle *h, struct dom_sid *sid = &(*sids)[i]; if (use_lookupnames4) { + if (i >= sid_array3.count) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + dom_idx = sid_array3.sids[i].sid_index; (*types)[i] = sid_array3.sids[i].sid_type; } else { + if (i >= sid_array.count) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + dom_idx = sid_array.sids[i].sid_index; (*types)[i] = sid_array.sids[i].sid_type; } @@ -677,6 +687,14 @@ NTSTATUS dcerpc_lsa_lookup_names_generic(struct dcerpc_binding_handle *h, (*types)[i] = SID_NAME_UNKNOWN; continue; } + if (domains == NULL) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (dom_idx >= domains->count) { + *presult = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } if (use_lookupnames4) { sid_copy(sid, sid_array3.sids[i].sid); diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c index 3fd93bc..3b1cdcf 100644 --- a/source3/rpc_server/netlogon/srv_netlog_nt.c +++ b/source3/rpc_server/netlogon/srv_netlog_nt.c @@ -586,7 +586,7 @@ static NTSTATUS samr_find_machine_account(TALLOC_CTX *mem_ctx, status = NT_STATUS_NO_SUCH_USER; goto out; } - if (rids.count != types.count) { + if (types.count != 1) { status = NT_STATUS_INVALID_PARAMETER; goto out; } diff --git a/source3/rpcclient/cmd_lsarpc.c b/source3/rpcclient/cmd_lsarpc.c index 9893a7c..83b76e9 100644 --- a/source3/rpcclient/cmd_lsarpc.c +++ b/source3/rpcclient/cmd_lsarpc.c @@ -323,7 +323,7 @@ static NTSTATUS cmd_lsa_lookup_names4(struct rpc_pipe_client *cli, uint32_t num_names; struct lsa_String *names; - struct lsa_RefDomainList *domains; + struct lsa_RefDomainList *domains = NULL; struct lsa_TransSidArray3 sids; uint32_t count = 0; int i; @@ -361,6 +361,10 @@ static NTSTATUS cmd_lsa_lookup_names4(struct rpc_pipe_client *cli, return result; } + if (sids.count != num_names) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + for (i = 0; i < sids.count; i++) { fstring sid_str; sid_to_fstring(sid_str, sids.sids[i].sid); diff --git a/source3/rpcclient/cmd_samr.c b/source3/rpcclient/cmd_samr.c index 727c9d1..0a6dede 100644 --- a/source3/rpcclient/cmd_samr.c +++ b/source3/rpcclient/cmd_samr.c @@ -385,7 +385,17 @@ static NTSTATUS cmd_samr_query_user(struct rpc_pipe_client *cli, if (!NT_STATUS_IS_OK(status)) { goto done; } + if (NT_STATUS_IS_OK(result)) { + if (rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + status = dcerpc_samr_OpenUser(b, mem_ctx, &domain_pol, access_mask, @@ -1450,6 +1460,15 @@ static NTSTATUS cmd_samr_delete_alias(struct rpc_pipe_client *cli, goto done; } if (NT_STATUS_IS_OK(result)) { + if (rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + status = dcerpc_samr_OpenAlias(b, mem_ctx, &domain_pol, access_mask, @@ -2112,6 +2131,14 @@ static NTSTATUS cmd_samr_lookup_names(struct rpc_pipe_client *cli, status = result; goto done; } + if (rids.count != num_names) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != num_names) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } /* Display results */ @@ -2269,6 +2296,14 @@ static NTSTATUS cmd_samr_delete_dom_group(struct rpc_pipe_client *cli, status = result; goto done; } + if (group_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } status = dcerpc_samr_OpenGroup(b, mem_ctx, &domain_pol, @@ -2372,6 +2407,14 @@ static NTSTATUS cmd_samr_delete_dom_user(struct rpc_pipe_client *cli, status = result; goto done; } + if (user_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } status = dcerpc_samr_OpenUser(b, mem_ctx, &domain_pol, @@ -2758,6 +2801,14 @@ static NTSTATUS cmd_samr_chgpasswd(struct rpc_pipe_client *cli, status = result; goto done; } + if (rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } status = dcerpc_samr_OpenUser(b, mem_ctx, &domain_pol, @@ -3161,7 +3212,12 @@ static NTSTATUS cmd_samr_setuserinfo_int(struct rpc_pipe_client *cli, if (!NT_STATUS_IS_OK(result)) { return result; } - + if (rids.count != 1) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (types.count != 1) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } status = dcerpc_samr_OpenUser(b, mem_ctx, &domain_pol, diff --git a/source3/smbd/lanman.c b/source3/smbd/lanman.c index f56ea30..aef12df 100644 --- a/source3/smbd/lanman.c +++ b/source3/smbd/lanman.c @@ -2628,6 +2628,14 @@ static bool api_NetUserGetGroups(struct smbd_server_connection *sconn, nt_errstr(result))); goto close_domain; } + if (rid.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto close_domain; + } + if (type.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto close_domain; + } if (type.ids[0] != SID_NAME_USER) { DEBUG(10, ("%s is a %s, not a user\n", UserName, diff --git a/source3/utils/net_rpc.c b/source3/utils/net_rpc.c index c0d52ed..582f450 100644 --- a/source3/utils/net_rpc.c +++ b/source3/utils/net_rpc.c @@ -1656,6 +1656,14 @@ static NTSTATUS rpc_group_delete_internals(struct net_context *c, d_fprintf(stderr, _("Lookup of '%s' failed\n"),argv[0]); goto done; } + if (group_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } switch (name_types.ids[0]) { @@ -2063,6 +2071,14 @@ static NTSTATUS rpc_add_groupmem(struct rpc_pipe_client *pipe_hnd, member); goto done; } + if (rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (rid_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } status = dcerpc_samr_OpenGroup(b, mem_ctx, &domain_pol, @@ -2318,6 +2334,14 @@ static NTSTATUS rpc_del_groupmem(struct net_context *c, member); goto done; } + if (rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (rid_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } status = dcerpc_samr_OpenGroup(b, mem_ctx, &domain_pol, @@ -3101,9 +3125,15 @@ static NTSTATUS rpc_group_members_internals(struct net_context *c, if (rids.count != 1) { d_fprintf(stderr, _("Couldn't find group %s\n"), argv[0]); - return result; + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (rid_types.count != 1) { + d_fprintf(stderr, _("Couldn't find group %s\n"), + argv[0]); + return NT_STATUS_INVALID_NETWORK_RESPONSE; } + if (rid_types.ids[0] == SID_NAME_DOM_GRP) { return rpc_list_group_members(c, pipe_hnd, mem_ctx, domain_name, domain_sid, &domain_pol, @@ -6018,6 +6048,14 @@ static NTSTATUS rpc_trustdom_del_internals(struct net_context *c, acct_name, nt_errstr(result) ); goto done; } + if (user_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } status = dcerpc_samr_OpenUser(b, mem_ctx, &domain_pol, diff --git a/source3/utils/net_rpc_join.c b/source3/utils/net_rpc_join.c index f2309f6..b1d6e91 100644 --- a/source3/utils/net_rpc_join.c +++ b/source3/utils/net_rpc_join.c @@ -367,6 +367,15 @@ int net_rpc_join_newstyle(struct net_context *c, int argc, const char **argv) ("error looking up rid for user %s: %s/%s\n", acct_name, nt_errstr(status), nt_errstr(result))); + if (user_rids.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.count != 1) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (name_types.ids[0] != SID_NAME_USER) { DEBUG(0, ("%s is not a user account (type=%d)\n", acct_name, name_types.ids[0])); goto done; diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c index ad4ce44..344f2dc 100644 --- a/source3/winbindd/winbindd_rpc.c +++ b/source3/winbindd/winbindd_rpc.c @@ -1039,7 +1039,7 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx, struct lsa_TransNameArray **pnames) { struct lsa_TransNameArray2 lsa_names2; - struct lsa_TransNameArray *names; + struct lsa_TransNameArray *names = *pnames; uint32_t i, count; NTSTATUS status, result; @@ -1064,10 +1064,6 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx, return NT_STATUS_INVALID_NETWORK_RESPONSE; } - names = TALLOC_ZERO_P(mem_ctx, struct lsa_TransNameArray); - if (names == NULL) { - return NT_STATUS_NO_MEMORY; - } names->count = lsa_names2.count; names->names = talloc_array(names, struct lsa_TranslatedName, names->count); @@ -1090,7 +1086,6 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx, return NT_STATUS_INVALID_NETWORK_RESPONSE; } } - *pnames = names; return result; } @@ -1100,7 +1095,7 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx, struct lsa_RefDomainList **pdomains, struct lsa_TransNameArray **pnames) { - struct lsa_TransNameArray *names; + struct lsa_TransNameArray *names = *pnames; struct rpc_pipe_client *cli = NULL; struct policy_handle lsa_policy; uint32_t count; @@ -1117,10 +1112,6 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx, pdomains, pnames); } - names = TALLOC_ZERO_P(mem_ctx, struct lsa_TransNameArray); - if (names == NULL) { - return NT_STATUS_NO_MEMORY; - } status = dcerpc_lsa_LookupSids(cli->binding_handle, mem_ctx, &lsa_policy, sids, pdomains, names, LSA_LOOKUP_NAMES_ALL, @@ -1148,6 +1139,5 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx, } } - *pnames = names; return result; } diff --git a/source4/libcli/util/clilsa.c b/source4/libcli/util/clilsa.c index d705abd..7fe3fe3 100644 --- a/source4/libcli/util/clilsa.c +++ b/source4/libcli/util/clilsa.c @@ -329,7 +329,11 @@ NTSTATUS smblsa_lookup_name(struct smbcli_state *cli, } if (sids.count != 1) { talloc_free(mem_ctx2); - return NT_STATUS_UNSUCCESSFUL; + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (domains->count != 1) { + talloc_free(mem_ctx2); + return NT_STATUS_INVALID_NETWORK_RESPONSE; } sid = domains->domains[0].sid; diff --git a/source4/libnet/groupinfo.c b/source4/libnet/groupinfo.c index 44c21ee..2b7963c 100644 --- a/source4/libnet/groupinfo.c +++ b/source4/libnet/groupinfo.c @@ -87,12 +87,16 @@ static void continue_groupinfo_lookup(struct tevent_req *subreq) s->monitor_fn(&msg); } - /* have we actually got name resolved - we're looking for only one at the moment */ - if (s->lookup.out.rids->count == 0) { - composite_error(c, NT_STATUS_NO_SUCH_USER); + if (s->lookup.out.rids->count != s->lookup.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } + if (s->lookup.out.types->count != s->lookup.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); + return; } /* TODO: find proper status code for more than one rid found */ diff --git a/source4/libnet/groupman.c b/source4/libnet/groupman.c index c8762f4..c8e0e06 100644 --- a/source4/libnet/groupman.c +++ b/source4/libnet/groupman.c @@ -212,13 +212,13 @@ static void continue_groupdel_name_found(struct tevent_req *subreq) /* what to do when there's no group account to delete and what if there's more than one rid resolved */ - if (!s->lookupname.out.rids->count) { - c->status = NT_STATUS_NO_SUCH_GROUP; + if (s->lookupname.out.rids->count != s->lookupname.in.num_names) { + c->status = NT_STATUS_INVALID_NETWORK_RESPONSE; composite_error(c, c->status); return; - - } else if (!s->lookupname.out.rids->count > 1) { - c->status = NT_STATUS_INVALID_ACCOUNT_NAME; + } + if (s->lookupname.out.types->count != s->lookupname.in.num_names) { + c->status = NT_STATUS_INVALID_NETWORK_RESPONSE; composite_error(c, c->status); return; } diff --git a/source4/libnet/libnet_join.c b/source4/libnet/libnet_join.c index 6e76df4..81ec403 100644 --- a/source4/libnet/libnet_join.c +++ b/source4/libnet/libnet_join.c @@ -656,9 +656,17 @@ NTSTATUS libnet_JoinDomain(struct libnet_context *ctx, TALLOC_CTX *mem_ctx, stru "samr_LookupNames for [%s] returns %d RIDs", r->in.account_name, ln.out.rids->count); talloc_free(tmp_ctx); - return NT_STATUS_INVALID_PARAMETER; + return NT_STATUS_INVALID_NETWORK_RESPONSE; } - + + if (ln.out.types->count != 1) { + r->out.error_string = talloc_asprintf(mem_ctx, + "samr_LookupNames for [%s] returns %d RID TYPEs", + r->in.account_name, ln.out.types->count); + talloc_free(tmp_ctx); + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + /* prepare samr_OpenUser */ ZERO_STRUCTP(u_handle); ou.in.domain_handle = &d_handle; diff --git a/source4/libnet/libnet_lookup.c b/source4/libnet/libnet_lookup.c index f937940..204978b 100644 --- a/source4/libnet/libnet_lookup.c +++ b/source4/libnet/libnet_lookup.c @@ -363,6 +363,11 @@ static void continue_name_found(struct tevent_req *subreq) c->status = s->lookup.out.result; if (!composite_is_ok(c)) return; + if (s->lookup.out.sids->count != s->lookup.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } + composite_done(c); } diff --git a/source4/libnet/libnet_passwd.c b/source4/libnet/libnet_passwd.c index 861d746..77176bc 100644 --- a/source4/libnet/libnet_passwd.c +++ b/source4/libnet/libnet_passwd.c @@ -627,10 +627,18 @@ static NTSTATUS libnet_SetPassword_samr(struct libnet_context *ctx, TALLOC_CTX * r->samr.out.error_string = talloc_asprintf(mem_ctx, "samr_LookupNames for [%s] returns %d RIDs", r->samr.in.account_name, ln.out.rids->count); - status = NT_STATUS_INVALID_PARAMETER; + status = NT_STATUS_INVALID_NETWORK_RESPONSE; goto disconnect; } + if (ln.out.types->count != 1) { + r->samr.out.error_string = talloc_asprintf(mem_ctx, + "samr_LookupNames for [%s] returns %d RID TYPEs", + r->samr.in.account_name, ln.out.types->count); + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto disconnect; + } + /* prepare samr_OpenUser */ ZERO_STRUCT(u_handle); ou.in.domain_handle = &d_handle; diff --git a/source4/libnet/userinfo.c b/source4/libnet/userinfo.c index 8d9c841..c77e9bf 100644 --- a/source4/libnet/userinfo.c +++ b/source4/libnet/userinfo.c @@ -90,8 +90,13 @@ static void continue_userinfo_lookup(struct tevent_req *subreq) /* have we actually got name resolved - we're looking for only one at the moment */ - if (s->lookup.out.rids->count == 0) { - composite_error(c, NT_STATUS_NO_SUCH_USER); + if (s->lookup.out.rids->count != s->lookup.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } + if (s->lookup.out.types->count != s->lookup.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); + return; } /* TODO: find proper status code for more than one rid found */ diff --git a/source4/libnet/userman.c b/source4/libnet/userman.c index 0f586b1..b4399ce 100644 --- a/source4/libnet/userman.c +++ b/source4/libnet/userman.c @@ -236,14 +236,12 @@ static void continue_userdel_name_found(struct tevent_req *subreq) /* what to do when there's no user account to delete and what if there's more than one rid resolved */ - if (!s->lookupname.out.rids->count) { - c->status = NT_STATUS_NO_SUCH_USER; - composite_error(c, c->status); + if (s->lookupname.out.rids->count != s->lookupname.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); return; - - } else if (!s->lookupname.out.rids->count > 1) { - c->status = NT_STATUS_INVALID_ACCOUNT_NAME; - composite_error(c, c->status); + } + if (s->lookupname.out.types->count != s->lookupname.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); return; } @@ -511,14 +509,12 @@ static void continue_usermod_name_found(struct tevent_req *subreq) /* what to do when there's no user account to delete and what if there's more than one rid resolved */ - if (!s->lookupname.out.rids->count) { - c->status = NT_STATUS_NO_SUCH_USER; - composite_error(c, c->status); + if (s->lookupname.out.rids->count != s->lookupname.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); return; - - } else if (!s->lookupname.out.rids->count > 1) { - c->status = NT_STATUS_INVALID_ACCOUNT_NAME; - composite_error(c, c->status); + } + if (s->lookupname.out.types->count != s->lookupname.in.num_names) { + composite_error(c, NT_STATUS_INVALID_NETWORK_RESPONSE); return; } diff --git a/source4/winbind/wb_async_helpers.c b/source4/winbind/wb_async_helpers.c index 01ff4a4..bb6dd27 100644 --- a/source4/winbind/wb_async_helpers.c +++ b/source4/winbind/wb_async_helpers.c @@ -283,6 +283,12 @@ static void lsa_lookupnames_recv_sids(struct tevent_req *subreq) return; } + if (state->sids.count != state->num_names) { + composite_error(state->ctx, + NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } + state->result = talloc_array(state, struct wb_sid_object *, state->num_names); if (composite_nomem(state->result, state->ctx)) return; @@ -301,9 +307,14 @@ static void lsa_lookupnames_recv_sids(struct tevent_req *subreq) continue; } + if (domains == NULL) { + composite_error(state->ctx, + NT_STATUS_INVALID_NETWORK_RESPONSE); + return; + } if (sid->sid_index >= domains->count) { composite_error(state->ctx, - NT_STATUS_INVALID_PARAMETER); + NT_STATUS_INVALID_NETWORK_RESPONSE); return; } -- 1.7.9.5 From 1494ef3403141455cd36f36813b165365fe35bdb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 19 Nov 2013 14:10:15 -0800 Subject: [PATCH 15/15] CVE-2013-4408:s3:Ensure LookupRids() replies arrays are range checked. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185 Signed-off-by: Jeremy Allison Signed-off-by: Stefan Metzmacher --- source3/lib/netapi/group.c | 16 ++++++++++++++++ source3/lib/netapi/user.c | 16 ++++++++++++++++ source3/rpcclient/cmd_samr.c | 8 ++++++++ source3/utils/net_rpc.c | 7 ++++++- source3/winbindd/winbindd_msrpc.c | 10 ++++++++-- source3/winbindd/winbindd_rpc.c | 10 ++++++++-- 6 files changed, 62 insertions(+), 5 deletions(-) diff --git a/source3/lib/netapi/group.c b/source3/lib/netapi/group.c index 360640f..09a0f0b 100644 --- a/source3/lib/netapi/group.c +++ b/source3/lib/netapi/group.c @@ -395,6 +395,14 @@ WERROR NetGroupDel_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (names.count != rid_array->count) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (member_types.count != rid_array->count) { + werr = WERR_BAD_NET_RESP; + goto done; + } } for (i=0; i < rid_array->count; i++) { @@ -1623,6 +1631,14 @@ WERROR NetGroupGetUsers_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (names.count != rid_array->count) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (member_types.count != rid_array->count) { + werr = WERR_BAD_NET_RESP; + goto done; + } for (i=0; i < names.count; i++) { diff --git a/source3/lib/netapi/user.c b/source3/lib/netapi/user.c index d16d226..7c21703 100644 --- a/source3/lib/netapi/user.c +++ b/source3/lib/netapi/user.c @@ -3113,6 +3113,14 @@ WERROR NetUserGetGroups_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (names.count != rid_array->count) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != rid_array->count) { + werr = WERR_BAD_NET_RESP; + goto done; + } for (i=0; i < names.count; i++) { status = add_GROUP_USERS_INFO_X_buffer(ctx, @@ -3716,6 +3724,14 @@ WERROR NetUserGetLocalGroups_r(struct libnetapi_ctx *ctx, werr = ntstatus_to_werror(result); goto done; } + if (names.count != num_rids) { + werr = WERR_BAD_NET_RESP; + goto done; + } + if (types.count != num_rids) { + werr = WERR_BAD_NET_RESP; + goto done; + } for (i=0; i < names.count; i++) { status = add_LOCALGROUP_USERS_INFO_X_buffer(ctx, diff --git a/source3/rpcclient/cmd_samr.c b/source3/rpcclient/cmd_samr.c index 0a6dede..f252acc 100644 --- a/source3/rpcclient/cmd_samr.c +++ b/source3/rpcclient/cmd_samr.c @@ -2220,6 +2220,14 @@ static NTSTATUS cmd_samr_lookup_rids(struct rpc_pipe_client *cli, goto done; /* Display results */ + if (num_rids != names.count) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } + if (num_rids != types.count) { + status = NT_STATUS_INVALID_NETWORK_RESPONSE; + goto done; + } for (i = 0; i < num_rids; i++) { printf("rid 0x%x: %s (%d)\n", diff --git a/source3/utils/net_rpc.c b/source3/utils/net_rpc.c index 582f450..fb67060 100644 --- a/source3/utils/net_rpc.c +++ b/source3/utils/net_rpc.c @@ -2889,7 +2889,12 @@ static NTSTATUS rpc_list_group_members(struct net_context *c, if (!NT_STATUS_IS_OK(result)) { return result; } - + if (names.count != this_time) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (types.count != this_time) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } /* We only have users as members, but make the output the same as the output of alias members */ diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c index b14a4f8..efebeb9 100644 --- a/source3/winbindd/winbindd_msrpc.c +++ b/source3/winbindd/winbindd_msrpc.c @@ -744,14 +744,20 @@ static NTSTATUS msrpc_lookup_groupmem(struct winbindd_domain *domain, /* Copy result into array. The talloc system will take care of freeing the temporary arrays later on. */ - if (tmp_names.count != tmp_types.count) { - return NT_STATUS_UNSUCCESSFUL; + if (tmp_names.count != num_lookup_rids) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (tmp_types.count != num_lookup_rids) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; } for (r=0; r= *num_names) { + break; + } (*names)[total_names] = fill_domain_username_talloc( mem_ctx, domain->name, tmp_names.names[r].string, true); diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c index 344f2dc..c560a6b 100644 --- a/source3/winbindd/winbindd_rpc.c +++ b/source3/winbindd/winbindd_rpc.c @@ -871,14 +871,20 @@ NTSTATUS rpc_lookup_groupmem(TALLOC_CTX *mem_ctx, /* Copy result into array. The talloc system will take care of freeing the temporary arrays later on. */ - if (tmp_names.count != tmp_types.count) { - return NT_STATUS_UNSUCCESSFUL; + if (tmp_names.count != num_names) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; + } + if (tmp_types.count != num_names) { + return NT_STATUS_INVALID_NETWORK_RESPONSE; } for (r = 0; r < tmp_names.count; r++) { if (tmp_types.ids[r] == SID_NAME_UNKNOWN) { continue; } + if (total_names >= num_names) { + break; + } names[total_names] = fill_domain_username_talloc(names, domain_name, tmp_names.names[r].string, -- 1.7.9.5 From f62683956a3b182f6a61cc7a2b4ada2e74cde243 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Wed, 16 Oct 2013 16:30:55 +0100 Subject: [PATCH] fail authentication for single group name which cannot be converted to sid furthermore if more than one name is supplied and no sid is converted then also fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10300 Signed-off-by: Noel Power Reviewed-by: Andreas Schneider Reviewed-by: David Disseldorp [ddiss@samba.org: fixed incorrect bugzilla tag I added to master commit] --- nsswitch/pam_winbind.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nsswitch/pam_winbind.c b/nsswitch/pam_winbind.c index 9322971..cd5e7ba 100644 --- a/nsswitch/pam_winbind.c +++ b/nsswitch/pam_winbind.c @@ -1172,6 +1172,12 @@ static bool winbind_name_list_to_sid_string_list(struct pwb_context *ctx, _make_remark_format(ctx, PAM_TEXT_INFO, _("Cannot convert group %s " "to sid, please contact your administrator to see " "if group %s is valid."), search_location, search_location); + + /* If no valid groups were converted we should fail outright */ + if (name_list != NULL && strlen(sid_list_buffer) == 0) { + result = false; + goto out; + } /* * The lookup of the last name failed.. * It results in require_member_of_sid ends with ',' -- 1.8.1.4