From 458b014a4d10e89f99cc0b54a815b50d6c817dce Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 8 Apr 2020 08:49:23 +1200 Subject: CVE-2020-10704: libcli ldap_message: Add search size limits to ldap_decode Add search request size limits to ldap_decode calls. The ldap server uses the smb.conf variable "ldap max search request size" which defaults to 250Kb. For cldap the limit is hard coded as 4096. Credit to OSS-Fuzz REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20454 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14334 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett --- docs-xml/smbdotconf/ldap/ldapmaxsearchrequest.xml | 18 +++++++++++++++++ lib/param/loadparm.c | 2 ++ libcli/cldap/cldap.c | 18 ++++++++++++++--- libcli/ldap/ldap_message.c | 1 + libcli/ldap/ldap_message.h | 5 +++++ libcli/ldap/tests/ldap_message_test.c | 24 +++++++++++++++++++---- source3/param/loadparm.c | 1 + source4/ldap_server/ldap_server.c | 10 ++++++++-- source4/libcli/ldap/ldap_client.c | 3 ++- 9 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 docs-xml/smbdotconf/ldap/ldapmaxsearchrequest.xml diff --git a/docs-xml/smbdotconf/ldap/ldapmaxsearchrequest.xml b/docs-xml/smbdotconf/ldap/ldapmaxsearchrequest.xml new file mode 100644 index 00000000000..ebeb0816c01 --- /dev/null +++ b/docs-xml/smbdotconf/ldap/ldapmaxsearchrequest.xml @@ -0,0 +1,18 @@ + + + + This parameter specifies the maximum permitted size (in bytes) + for an LDAP search request. + + + + If the request size exceeds this limit the request will be + rejected. + + +256000 +4194304 + diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c index 8e3521c918e..e0c6adec9c8 100644 --- a/lib/param/loadparm.c +++ b/lib/param/loadparm.c @@ -3029,6 +3029,8 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx) lp_ctx, "ldap max anonymous request size", "256000"); lpcfg_do_global_parameter( lp_ctx, "ldap max authenticated request size", "16777216"); + lpcfg_do_global_parameter( + lp_ctx, "ldap max search request size", "256000"); for (i = 0; parm_table[i].label; i++) { if (!(lp_ctx->flags[i] & FLAG_CMDLINE)) { diff --git a/libcli/cldap/cldap.c b/libcli/cldap/cldap.c index 8fa9ce0b273..25c1b40f8d9 100644 --- a/libcli/cldap/cldap.c +++ b/libcli/cldap/cldap.c @@ -111,6 +111,11 @@ struct cldap_search_state { struct tevent_req *req; }; +/* + * For CLDAP we limit the maximum search request size to 4kb + */ +#define MAX_SEARCH_REQUEST 4096 + static int cldap_socket_destructor(struct cldap_socket *c) { while (c->searches.list) { @@ -224,6 +229,9 @@ static bool cldap_socket_recv_dgram(struct cldap_socket *c, void *p; struct cldap_search_state *search; NTSTATUS status; + struct ldap_request_limits limits = { + .max_search_size = MAX_SEARCH_REQUEST + }; if (in->recv_errno != 0) { goto error; @@ -242,7 +250,7 @@ static bool cldap_socket_recv_dgram(struct cldap_socket *c, } /* this initial decode is used to find the message id */ - status = ldap_decode(asn1, NULL, in->ldap_msg); + status = ldap_decode(asn1, &limits, NULL, in->ldap_msg); if (!NT_STATUS_IS_OK(status)) { goto nterror; } @@ -770,6 +778,9 @@ NTSTATUS cldap_search_recv(struct tevent_req *req, struct cldap_search_state); struct ldap_message *ldap_msg; NTSTATUS status; + struct ldap_request_limits limits = { + .max_search_size = MAX_SEARCH_REQUEST + }; if (tevent_req_is_nterror(req, &status)) { goto failed; @@ -780,7 +791,7 @@ NTSTATUS cldap_search_recv(struct tevent_req *req, goto nomem; } - status = ldap_decode(state->response.asn1, NULL, ldap_msg); + status = ldap_decode(state->response.asn1, &limits, NULL, ldap_msg); if (!NT_STATUS_IS_OK(status)) { goto failed; } @@ -796,7 +807,8 @@ NTSTATUS cldap_search_recv(struct tevent_req *req, *io->out.response = ldap_msg->r.SearchResultEntry; /* decode the 2nd part */ - status = ldap_decode(state->response.asn1, NULL, ldap_msg); + status = ldap_decode( + state->response.asn1, &limits, NULL, ldap_msg); if (!NT_STATUS_IS_OK(status)) { goto failed; } diff --git a/libcli/ldap/ldap_message.c b/libcli/ldap/ldap_message.c index ba82bddeab1..d38fa0b3b61 100644 --- a/libcli/ldap/ldap_message.c +++ b/libcli/ldap/ldap_message.c @@ -1162,6 +1162,7 @@ static bool ldap_decode_attribs(TALLOC_CTX *mem_ctx, struct asn1_data *data, /* This routine returns LDAP status codes */ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, + const struct ldap_request_limits *limits, const struct ldap_control_handler *control_handlers, struct ldap_message *msg) { diff --git a/libcli/ldap/ldap_message.h b/libcli/ldap/ldap_message.h index 2f64881c053..19bfb99ac97 100644 --- a/libcli/ldap/ldap_message.h +++ b/libcli/ldap/ldap_message.h @@ -213,10 +213,15 @@ struct ldap_control_handler { bool (*encode)(void *mem_ctx, void *in, DATA_BLOB *out); }; +struct ldap_request_limits { + unsigned max_search_size; +}; + struct asn1_data; struct ldap_message *new_ldap_message(TALLOC_CTX *mem_ctx); NTSTATUS ldap_decode(struct asn1_data *data, + const struct ldap_request_limits *limits, const struct ldap_control_handler *control_handlers, struct ldap_message *msg); bool ldap_encode(struct ldap_message *msg, diff --git a/libcli/ldap/tests/ldap_message_test.c b/libcli/ldap/tests/ldap_message_test.c index 9cc9cc5d8a0..c5aacd4bc6b 100644 --- a/libcli/ldap/tests/ldap_message_test.c +++ b/libcli/ldap/tests/ldap_message_test.c @@ -117,6 +117,9 @@ static void test_empty_input(void **state) NTSTATUS status; uint8_t buf[0]; size_t len = 0; + struct ldap_request_limits limits = { + .max_search_size = 256000, + }; asn1 = asn1_init(test_ctx, ASN1_MAX_TREE_DEPTH); @@ -127,7 +130,8 @@ static void test_empty_input(void **state) ldap_msg = talloc(test_ctx, struct ldap_message); assert_non_null(ldap_msg); - status = ldap_decode(asn1, samba_ldap_control_handlers(), ldap_msg); + status = ldap_decode( + asn1, &limits, samba_ldap_control_handlers(), ldap_msg); assert_ldap_status_equal(LDAP_PROTOCOL_ERROR, status); } @@ -149,6 +153,9 @@ static void test_recursion_depth_large(void **state) uint8_t *buffer = NULL; const size_t BUFF_SIZE = 1048576; size_t len; + struct ldap_request_limits limits = { + .max_search_size = 256000, + }; /* @@ -169,7 +176,8 @@ static void test_recursion_depth_large(void **state) ldap_msg = talloc(test_ctx, struct ldap_message); assert_non_null(ldap_msg); - status = ldap_decode(asn1, samba_ldap_control_handlers(), ldap_msg); + status = ldap_decode( + asn1, &limits, samba_ldap_control_handlers(), ldap_msg); assert_ldap_status_equal(LDAP_PROTOCOL_ERROR, status); } @@ -189,6 +197,9 @@ static void test_recursion_depth_equals_max(void **state) uint8_t *buffer = NULL; const size_t BUFF_SIZE = 1048576; size_t len; + struct ldap_request_limits limits = { + .max_search_size = 256000, + }; buffer = talloc_zero_array(test_ctx, uint8_t, BUFF_SIZE); @@ -205,7 +216,8 @@ static void test_recursion_depth_equals_max(void **state) ldap_msg = talloc(test_ctx, struct ldap_message); assert_non_null(ldap_msg); - status = ldap_decode(asn1, samba_ldap_control_handlers(), ldap_msg); + status = ldap_decode( + asn1, &limits, samba_ldap_control_handlers(), ldap_msg); assert_true(NT_STATUS_IS_OK(status)); } @@ -225,6 +237,9 @@ static void test_recursion_depth_greater_than_max(void **state) uint8_t *buffer = NULL; const size_t BUFF_SIZE = 1048576; size_t len; + struct ldap_request_limits limits = { + .max_search_size = 256000, + }; buffer = talloc_zero_array(test_ctx, uint8_t, BUFF_SIZE); @@ -241,7 +256,8 @@ static void test_recursion_depth_greater_than_max(void **state) ldap_msg = talloc(test_ctx, struct ldap_message); assert_non_null(ldap_msg); - status = ldap_decode(asn1, samba_ldap_control_handlers(), ldap_msg); + status = ldap_decode( + asn1, &limits, samba_ldap_control_handlers(), ldap_msg); assert_ldap_status_equal(LDAP_PROTOCOL_ERROR, status); } diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index dc1ad7aeafb..5ee6d33c7fc 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -958,6 +958,7 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals) Globals.ldap_max_anonymous_request_size = 256000; Globals.ldap_max_authenticated_request_size = 16777216; + Globals.ldap_max_search_request_size = 256000; /* Now put back the settings that were set with lp_set_cmdline() */ apply_lp_set_cmdline(); diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c index a730667abb9..a9b162b284e 100644 --- a/source4/ldap_server/ldap_server.c +++ b/source4/ldap_server/ldap_server.c @@ -538,6 +538,7 @@ static void ldapsrv_call_read_done(struct tevent_req *subreq) struct asn1_data *asn1; DATA_BLOB blob; int ret = LDAP_SUCCESS; + struct ldap_request_limits limits = {0}; conn->sockets.read_req = NULL; @@ -593,8 +594,13 @@ static void ldapsrv_call_read_done(struct tevent_req *subreq) return; } - status = ldap_decode(asn1, samba_ldap_control_handlers(), - call->request); + limits.max_search_size = + lpcfg_ldap_max_search_request_size(conn->lp_ctx); + status = ldap_decode( + asn1, + &limits, + samba_ldap_control_handlers(), + call->request); if (!NT_STATUS_IS_OK(status)) { ldapsrv_terminate_connection(conn, nt_errstr(status)); return; diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c index 2d75af6af6e..cf276679594 100644 --- a/source4/libcli/ldap/ldap_client.c +++ b/source4/libcli/ldap/ldap_client.c @@ -277,6 +277,7 @@ static void ldap_connection_recv_done(struct tevent_req *subreq) struct ldap_message *msg; struct asn1_data *asn1; DATA_BLOB blob; + struct ldap_request_limits limits = {0}; msg = talloc_zero(conn, struct ldap_message); if (msg == NULL) { @@ -306,7 +307,7 @@ static void ldap_connection_recv_done(struct tevent_req *subreq) asn1_load_nocopy(asn1, blob.data, blob.length); - status = ldap_decode(asn1, samba_ldap_control_handlers(), msg); + status = ldap_decode(asn1, &limits, samba_ldap_control_handlers(), msg); asn1_free(asn1); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(msg); -- cgit v1.2.1