268 lines
8.8 KiB
Diff
268 lines
8.8 KiB
Diff
From 23e9eb71052e02aecf726609db0256c0d93e0b57 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 18/22] CVE-2020-10745: 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 retains the logic of e6e2ec0001fe3c010445e26cc0efddbc1f73416b.
|
|
|
|
Also DNS compression cannot be turned off for NBT.
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378
|
|
|
|
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
|
|
---
|
|
librpc/ndr/ndr_dns.c | 3 +-
|
|
librpc/ndr/ndr_dns_utils.c | 42 ++++++++++++++++---
|
|
librpc/ndr/ndr_dns_utils.h | 3 +-
|
|
librpc/ndr/ndr_nbt.c | 72 ++++----------------------------
|
|
librpc/wscript_build | 3 +-
|
|
selftest/knownfail.d/dns_packet | 1 -
|
|
selftest/knownfail.d/ndr_dns_nbt | 2 -
|
|
7 files changed, 49 insertions(+), 77 deletions(-)
|
|
delete mode 100644 selftest/knownfail.d/ndr_dns_nbt
|
|
|
|
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..325d9c68bea 100644
|
|
--- a/librpc/ndr/ndr_dns_utils.c
|
|
+++ b/librpc/ndr/ndr_dns_utils.c
|
|
@@ -9,9 +9,32 @@
|
|
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;
|
|
+ size_t max_length;
|
|
+ if (is_nbt) {
|
|
+ use_compression = true;
|
|
+ /*
|
|
+ * Max length is longer in NBT/Wins, because Windows counts
|
|
+ * the semi-decompressed size of the netbios name (16 bytes)
|
|
+ * rather than the wire size of 32, which is what you'd expect
|
|
+ * if it followed RFC1002 (it uses the short form in
|
|
+ * [MS-WINSRA]). In other words the maximum size of the
|
|
+ * "scope" is 237, not 221.
|
|
+ *
|
|
+ * We make the size limit slightly larger than 255 + 16,
|
|
+ * because the 237 scope limit is already enforced in the
|
|
+ * winsserver code with a specific return value; bailing out
|
|
+ * here would muck with that.
|
|
+ */
|
|
+ max_length = 274;
|
|
+ } else {
|
|
+ use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION);
|
|
+ max_length = 255;
|
|
+ }
|
|
|
|
if (!(ndr_flags & NDR_SCALARS)) {
|
|
return NDR_ERR_SUCCESS;
|
|
@@ -23,7 +46,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 +89,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 +106,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));
|
|
}
|
|
@@ -89,9 +120,10 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
|
|
if (*s == '.') {
|
|
s++;
|
|
}
|
|
- if (s - start > 255) {
|
|
+ if (s - start > max_length) {
|
|
return ndr_push_error(ndr, NDR_ERR_STRING,
|
|
- "name > 255 character long");
|
|
+ "name > %zu character long",
|
|
+ max_length);
|
|
}
|
|
}
|
|
|
|
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);
|
|
diff --git a/librpc/ndr/ndr_nbt.c b/librpc/ndr/ndr_nbt.c
|
|
index 838f947a168..e8dd7549a53 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 c165500644b..4917928a9c4 100644
|
|
--- a/librpc/wscript_build
|
|
+++ b/librpc/wscript_build
|
|
@@ -401,7 +401,7 @@ bld.SAMBA_SUBSYSTEM('NDR_SCHANNEL',
|
|
|
|
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',
|
|
@@ -666,6 +666,5 @@ bld.SAMBA_BINARY('test_ndr_dns_nbt',
|
|
cmocka
|
|
ndr
|
|
ndr_nbt
|
|
- NDR_DNS
|
|
''',
|
|
install=False)
|
|
diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet
|
|
index 0662266f689..e69de29bb2d 100644
|
|
--- a/selftest/knownfail.d/dns_packet
|
|
+++ b/selftest/knownfail.d/dns_packet
|
|
@@ -1 +0,0 @@
|
|
-samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components
|
|
diff --git a/selftest/knownfail.d/ndr_dns_nbt b/selftest/knownfail.d/ndr_dns_nbt
|
|
deleted file mode 100644
|
|
index 603395c8c50..00000000000
|
|
--- a/selftest/knownfail.d/ndr_dns_nbt
|
|
+++ /dev/null
|
|
@@ -1,2 +0,0 @@
|
|
-librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_all_dots
|
|
-librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_half_dots
|
|
--
|
|
2.17.1
|
|
|