The Samba-Bugzilla – Attachment 15982 Details for
Bug 14378
CVE-2020-10745 [SECURITY] invalid DNS or NBT queries containing dots use several seconds of CPU each
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
draft sufficient patch
wip-cve-2020-10745.patch (text/plain), 16.95 KB, created by
Douglas Bagnall
on 2020-05-15 01:56:17 UTC
(
hide
)
Description:
draft sufficient patch
Filename:
MIME Type:
Creator:
Douglas Bagnall
Created:
2020-05-15 01:56:17 UTC
Size:
16.95 KB
patch
obsolete
>From 08b02a50570250f9891baef8c10fa7ee843afe59 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Sat, 25 Apr 2020 11:02:08 +1200 >Subject: [PATCH 1/6] ndr_dns: move ndr_push_dns_string_list core into sharable > function > >This is because ndr_nbt.c does almost exactly the same thing with almost >exactly the same code, and they both do it wrong. Soon they will both >be using this better version. > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns.c | 79 +++------------------------------- > librpc/ndr/ndr_dns_utils.c | 88 ++++++++++++++++++++++++++++++++++++++ > librpc/ndr/ndr_dns_utils.h | 5 +++ > librpc/wscript_build | 2 +- > 4 files changed, 99 insertions(+), 75 deletions(-) > create mode 100644 librpc/ndr/ndr_dns_utils.c > create mode 100644 librpc/ndr/ndr_dns_utils.h > >diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c >index d37c8cc2ece..68a3c9de782 100644 >--- a/librpc/ndr/ndr_dns.c >+++ b/librpc/ndr/ndr_dns.c >@@ -33,6 +33,7 @@ > #include "librpc/gen_ndr/ndr_dnsp.h" > #include "system/locale.h" > #include "lib/util/util_net.h" >+#include "ndr_dns_utils.h" > > /* don't allow an unlimited number of name components */ > #define MAX_COMPONENTS 128 >@@ -159,80 +160,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, > int ndr_flags, > const char *s) > { >- if (!(ndr_flags & NDR_SCALARS)) { >- return NDR_ERR_SUCCESS; >- } >- >- while (s && *s) { >- enum ndr_err_code ndr_err; >- char *compname; >- size_t complen; >- uint32_t offset; >- >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >- /* see if we have pushed the remaining string already, >- * if so we use a label pointer to this string >- */ >- ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s, >- &offset, >- (comparison_fn_t)strcmp, >- false); >- if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- uint8_t b[2]; >- >- if (offset > 0x3FFF) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "offset for dns string " \ >- "label pointer " \ >- "%u[%08X] > 0x00003FFF", >- offset, offset); >- } >- >- b[0] = 0xC0 | (offset>>8); >- b[1] = (offset & 0xFF); >- >- return ndr_push_bytes(ndr, b, 2); >- } >- } >- >- complen = strcspn(s, "."); >- >- /* we need to make sure the length fits into 6 bytes */ >- if (complen > 0x3F) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "component length %u[%08X] > " \ >- "0x0000003F", >- (unsigned)complen, >- (unsigned)complen); >- } >- >- compname = talloc_asprintf(ndr, "%c%*.*s", >- (unsigned char)complen, >- (unsigned char)complen, >- (unsigned char)complen, s); >- NDR_ERR_HAVE_NO_MEMORY(compname); >- >- /* remember the current component + the rest of the string >- * so it can be reused later >- */ >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >- NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s, >- ndr->offset)); >- } >- >- /* push just this component into the blob */ >- NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, >- complen+1)); >- talloc_free(compname); >- >- s += complen; >- if (*s == '.') s++; >- } >- >- /* if we reach the end of the string and have pushed the last component >- * without using a label pointer, we need to terminate the string >- */ >- return ndr_push_bytes(ndr, (const uint8_t *)"", 1); >+ return ndr_push_dns_string_list(ndr, >+ &ndr->dns_string_list, >+ ndr_flags, >+ s); > } > > _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >new file mode 100644 >index 00000000000..2d9b5f1bc1e >--- /dev/null >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -0,0 +1,88 @@ >+#include "includes.h" >+#include "../librpc/ndr/libndr.h" >+#include "ndr_dns_utils.h" >+ >+ >+/** >+ push a dns/nbt string list to the wire >+*/ >+enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, >+ struct ndr_token_list *string_list, >+ int ndr_flags, >+ const char *s) >+{ >+ if (!(ndr_flags & NDR_SCALARS)) { >+ return NDR_ERR_SUCCESS; >+ } >+ >+ while (s && *s) { >+ enum ndr_err_code ndr_err; >+ char *compname; >+ size_t complen; >+ uint32_t offset; >+ >+ if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ /* see if we have pushed the remaining string already, >+ * if so we use a label pointer to this string >+ */ >+ ndr_err = ndr_token_retrieve_cmp_fn(string_list, s, >+ &offset, >+ (comparison_fn_t)strcmp, >+ false); >+ if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >+ uint8_t b[2]; >+ >+ if (offset > 0x3FFF) { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "offset for dns string " \ >+ "label pointer " \ >+ "%u[%08X] > 0x00003FFF", >+ offset, offset); >+ } >+ >+ b[0] = 0xC0 | (offset>>8); >+ b[1] = (offset & 0xFF); >+ >+ return ndr_push_bytes(ndr, b, 2); >+ } >+ } >+ >+ complen = strcspn(s, "."); >+ >+ /* we need to make sure the length fits into 6 bytes */ >+ if (complen > 0x3F) { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "component length %u[%08X] > " \ >+ "0x0000003F", >+ (unsigned)complen, >+ (unsigned)complen); >+ } >+ >+ compname = talloc_asprintf(ndr, "%c%*.*s", >+ (unsigned char)complen, >+ (unsigned char)complen, >+ (unsigned char)complen, s); >+ NDR_ERR_HAVE_NO_MEMORY(compname); >+ >+ /* remember the current component + the rest of the string >+ * so it can be reused later >+ */ >+ if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ NDR_CHECK(ndr_token_store(ndr, string_list, s, >+ ndr->offset)); >+ } >+ >+ /* push just this component into the blob */ >+ NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, >+ complen+1)); >+ talloc_free(compname); >+ >+ s += complen; >+ if (*s == '.') s++; >+ } >+ >+ /* if we reach the end of the string and have pushed the last component >+ * without using a label pointer, we need to terminate the string >+ */ >+ return ndr_push_bytes(ndr, (const uint8_t *)"", 1); >+} >diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h >new file mode 100644 >index 00000000000..823e3201112 >--- /dev/null >+++ b/librpc/ndr/ndr_dns_utils.h >@@ -0,0 +1,5 @@ >+ >+enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, >+ struct ndr_token_list *string_list, >+ int ndr_flags, >+ const char *s); >diff --git a/librpc/wscript_build b/librpc/wscript_build >index 928c96dbae4..c6b0c5bb786 100644 >--- a/librpc/wscript_build >+++ b/librpc/wscript_build >@@ -31,7 +31,7 @@ bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER', > ) > > bld.SAMBA_SUBSYSTEM('NDR_DNS', >- source='gen_ndr/ndr_dns.c ndr/ndr_dns.c', >+ source='gen_ndr/ndr_dns.c ndr/ndr_dns.c ndr/ndr_dns_utils.c', > public_deps='ndr NDR_DNSP' > ) > >-- >2.17.1 > > >From 167f7319fbf44f3d516b3e4bde857c3d9836d979 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Sat, 25 Apr 2020 11:03:30 +1200 >Subject: [PATCH 2/6] ndr/dns_utils: correct a comment > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns_utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index 2d9b5f1bc1e..2ce300863bc 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -49,7 +49,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > > complen = strcspn(s, "."); > >- /* we need to make sure the length fits into 6 bytes */ >+ /* the length must fit into 6 bits (i.e. <= 63) */ > if (complen > 0x3F) { > return ndr_push_error(ndr, NDR_ERR_STRING, > "component length %u[%08X] > " \ >-- >2.17.1 > > >From b0c260fea4ebb7c69d3ae1fd02d9dbcbde169e24 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Sat, 25 Apr 2020 11:10:18 +1200 >Subject: [PATCH 3/6] ndr_dns: do not allow consecutive dots > >The empty subdomain component is reserved for the root domain, which we >should only (and always) see at the end of the list. That is, we expect >"example.com.", but not "example..com". > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns_utils.c | 6 ++++++ > 1 file changed, 6 insertions(+) > >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index 2ce300863bc..6931dac422d 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -58,6 +58,12 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > (unsigned)complen); > } > >+ if (complen == 0 && s[complen] == '.') { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "component length is 0 " >+ "(consecutive dots)"); >+ } >+ > compname = talloc_asprintf(ndr, "%c%*.*s", > (unsigned char)complen, > (unsigned char)complen, >-- >2.17.1 > > >From 19e85736369ac86a301abe10017611b12e5a4e45 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 15 May 2020 00:06:08 +1200 >Subject: [PATCH 4/6] push: do not allow strings longer than 255 bytes > >--- > librpc/ndr/ndr_dns_utils.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index 6931dac422d..b7f11dbab4e 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -11,6 +11,8 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > int ndr_flags, > const char *s) > { >+ const char *start = s; >+ > if (!(ndr_flags & NDR_SCALARS)) { > return NDR_ERR_SUCCESS; > } >@@ -84,7 +86,13 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > talloc_free(compname); > > s += complen; >- if (*s == '.') s++; >+ if (*s == '.') { >+ s++; >+ } >+ if (s - start > 255) { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "name > 255 character long"); >+ } > } > > /* if we reach the end of the string and have pushed the last component >-- >2.17.1 > > >From 66316b04b60e30106c6b268c96e651ab33eecce3 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 15 May 2020 10:52:45 +1200 >Subject: [PATCH 5/6] ndr/dns-utils: prepare for NBT compatibility > >NBT has a funny thing where it sometimes needs to send a trailing dot as >part of the last component, because the string representation is a user >name. In DNS, "example.com", and "example.com." are the same, both >having three components ("example", "com", ""); in NBT, we want to treat >them differently, with the second form having the three components >("example", "com.", ""). > >This is based on e6e2ec0001fe3c010445e26cc0efddbc1f73416b. > >Also DNS compression cannot be turned off for NBT. > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns.c | 3 ++- > librpc/ndr/ndr_dns_utils.c | 21 ++++++++++++++++++--- > librpc/ndr/ndr_dns_utils.h | 3 ++- > 3 files changed, 22 insertions(+), 5 deletions(-) > >diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c >index 68a3c9de782..966e0b59786 100644 >--- a/librpc/ndr/ndr_dns.c >+++ b/librpc/ndr/ndr_dns.c >@@ -163,7 +163,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, > return ndr_push_dns_string_list(ndr, > &ndr->dns_string_list, > ndr_flags, >- s); >+ s, >+ false); > } > > _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index b7f11dbab4e..b054091c46c 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -9,9 +9,16 @@ > enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > struct ndr_token_list *string_list, > int ndr_flags, >- const char *s) >+ const char *s, >+ bool is_nbt) > { > const char *start = s; >+ bool use_compression; >+ if (is_nbt) { >+ use_compression = true; >+ } else { >+ use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION); >+ } > > if (!(ndr_flags & NDR_SCALARS)) { > return NDR_ERR_SUCCESS; >@@ -23,7 +30,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > size_t complen; > uint32_t offset; > >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ if (use_compression) { > /* see if we have pushed the remaining string already, > * if so we use a label pointer to this string > */ >@@ -66,6 +73,14 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > "(consecutive dots)"); > } > >+ if (is_nbt && s[complen] == '.' && s[complen + 1] == '\0') { >+ /* nbt names are sometimes usernames, and we need to >+ * keep a trailing dot to ensure it is byte-identical, >+ * (not just semantically identical given DNS >+ * semantics). */ >+ complen++; >+ } >+ > compname = talloc_asprintf(ndr, "%c%*.*s", > (unsigned char)complen, > (unsigned char)complen, >@@ -75,7 +90,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > /* remember the current component + the rest of the string > * so it can be reused later > */ >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ if (use_compression) { > NDR_CHECK(ndr_token_store(ndr, string_list, s, > ndr->offset)); > } >diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h >index 823e3201112..71a65433bbb 100644 >--- a/librpc/ndr/ndr_dns_utils.h >+++ b/librpc/ndr/ndr_dns_utils.h >@@ -2,4 +2,5 @@ > enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > struct ndr_token_list *string_list, > int ndr_flags, >- const char *s); >+ const char *s, >+ bool is_nbt); >-- >2.17.1 > > >From e2a493ff004d7bae4002115fafeb45204958a6fa Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 15 May 2020 11:48:27 +1200 >Subject: [PATCH 6/6] ndr/nbt_push_string: use ndr_dns_utils common push > function > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_nbt.c | 72 +++++--------------------------------------- > librpc/wscript_build | 2 +- > 2 files changed, 8 insertions(+), 66 deletions(-) > >diff --git a/librpc/ndr/ndr_nbt.c b/librpc/ndr/ndr_nbt.c >index b15dc5c06e5..8ed9f0a5f05 100644 >--- a/librpc/ndr/ndr_nbt.c >+++ b/librpc/ndr/ndr_nbt.c >@@ -25,6 +25,8 @@ > #include "includes.h" > #include "../libcli/nbt/libnbt.h" > #include "../libcli/netlogon/netlogon.h" >+#include "ndr_dns_utils.h" >+ > > /* don't allow an unlimited number of name components */ > #define MAX_COMPONENTS 128 >@@ -141,71 +143,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_nbt_string(struct ndr_pull *ndr, int ndr_fla > */ > _PUBLIC_ enum ndr_err_code ndr_push_nbt_string(struct ndr_push *ndr, int ndr_flags, const char *s) > { >- if (!(ndr_flags & NDR_SCALARS)) { >- return NDR_ERR_SUCCESS; >- } >- >- while (s && *s) { >- enum ndr_err_code ndr_err; >- char *compname; >- size_t complen; >- uint32_t offset; >- >- /* see if we have pushed the remaining string already, >- * if so we use a label pointer to this string >- */ >- ndr_err = ndr_token_retrieve_cmp_fn(&ndr->nbt_string_list, s, &offset, (comparison_fn_t)strcmp, false); >- if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- uint8_t b[2]; >- >- if (offset > 0x3FFF) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "offset for nbt string label pointer %u[%08X] > 0x00003FFF", >- offset, offset); >- } >- >- b[0] = 0xC0 | (offset>>8); >- b[1] = (offset & 0xFF); >- >- return ndr_push_bytes(ndr, b, 2); >- } >- >- complen = strcspn(s, "."); >- >- /* we need to make sure the length fits into 6 bytes */ >- if (complen > 0x3F) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "component length %u[%08X] > 0x0000003F", >- (unsigned)complen, (unsigned)complen); >- } >- >- if (s[complen] == '.' && s[complen+1] == '\0') { >- complen++; >- } >- >- compname = talloc_asprintf(ndr, "%c%*.*s", >- (unsigned char)complen, >- (unsigned char)complen, >- (unsigned char)complen, s); >- NDR_ERR_HAVE_NO_MEMORY(compname); >- >- /* remember the current componemt + the rest of the string >- * so it can be reused later >- */ >- NDR_CHECK(ndr_token_store(ndr, &ndr->nbt_string_list, s, ndr->offset)); >- >- /* push just this component into the blob */ >- NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, complen+1)); >- talloc_free(compname); >- >- s += complen; >- if (*s == '.') s++; >- } >- >- /* if we reach the end of the string and have pushed the last component >- * without using a label pointer, we need to terminate the string >- */ >- return ndr_push_bytes(ndr, (const uint8_t *)"", 1); >+ return ndr_push_dns_string_list(ndr, >+ &ndr->dns_string_list, >+ ndr_flags, >+ s, >+ true); > } > > >diff --git a/librpc/wscript_build b/librpc/wscript_build >index c6b0c5bb786..e9963474de0 100644 >--- a/librpc/wscript_build >+++ b/librpc/wscript_build >@@ -415,7 +415,7 @@ bld.SAMBA_SUBSYSTEM('NDR_NBT', > > bld.SAMBA_LIBRARY('ndr_nbt', > source='gen_ndr/ndr_nbt.c ndr/ndr_nbt.c', >- public_deps='ndr NDR_NBT_BUF NDR_SECURITY', >+ public_deps='ndr NDR_NBT_BUF NDR_SECURITY NDR_DNS', > public_headers='gen_ndr/nbt.h gen_ndr/ndr_nbt.h ndr/ndr_nbt.h', > header_path=[ ('gen_ndr*', 'gen_ndr'), ('ndr*', 'ndr')], > pc_files='ndr_nbt.pc', >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 14378
:
15978
|
15982
|
15999
|
16000
|
16001
|
16015
|
16016
|
16034
|
16035
|
16036
|
16037
|
16038
|
16039
|
16040
|
16041
|
16042
|
16045
|
16046
|
16047
|
16048
|
16049
|
16050
|
16051
|
16052
|
16053
|
16057
|
16068
|
16090
|
16093
|
16099