[SCM] Samba Shared Repository - branch master updated
Andrew Bartlett
abartlet at samba.org
Fri Feb 7 10:20:03 UTC 2020
The branch, master has been updated
via 3bc7acc6264 nmblib: avoid undefined behaviour in handle_name_ptrs()
via 91d4e79c279 librpc ndr: Change loop index to size_t
via 490bbb96b9b libprc ndr tests: Fix ndrdump test ntlmssp_CHALLENGE_MESSAGE
via 14182350f83 librpc ndr: ndr_pull_advance check for unsigned overflow.
via d1277f4d027 librpc ndr tests: Unsigned overflow in ndr_pull_advance
via 6d05fb3ea77 librpc ndr: NDR_PULL_ALIGN check for unsigned overflow
via 46edde86478 librpc ndr tests: uint32 overflow in NDR_PULL_ALIGN
via ae6927e4f08 librpc ndr: Heap-buffer-overflow in lzxpress_decompress
from c8e3c78d4f2 selftest: Test behaviour of DNS scavenge with an existing dNSTombstoned value
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 3bc7acc62646b105b03fd3c65e9170a373f95392
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Sun Jan 19 15:08:58 2020 +1300
nmblib: avoid undefined behaviour in handle_name_ptrs()
If *offset is length - 1, we would read ubuf[(*offset)+1] as the lower
bits of the new *offset. This value is undefined, but because it is
checked against the valid range, there is no way to read further
beyond that one byte.
Credit to oss-fuzz.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14242
OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20193
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
Autobuild-Date(master): Fri Feb 7 10:19:39 UTC 2020 on sn-devel-184
commit 91d4e79c279283dd6fc953a274b02b1957db84d8
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed Jan 22 14:18:00 2020 +1300
librpc ndr: Change loop index to size_t
Change the loop index in ndr_check_padding to size_t.
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 490bbb96b9bd082a190a590a6137f0e24f80a6e4
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri Feb 7 10:50:07 2020 +1300
libprc ndr tests: Fix ndrdump test ntlmssp_CHALLENGE_MESSAGE
Fix the expected data in fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt, as it
contained source code line numbers.
Andrew this test needs to be altered to us a regular expression and
remove the dependency on source line numbers.
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 14182350f8397d27d7642dae595dc52691f0acfe
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed Jan 15 12:37:06 2020 +1300
librpc ndr: ndr_pull_advance check for unsigned overflow.
Handle uint32 overflow in ndr_pull_advance
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit d1277f4d02701ac77f8538af353479b52aa81157
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Mon Jan 27 10:06:55 2020 +1300
librpc ndr tests: Unsigned overflow in ndr_pull_advance
Check that uint32 overflow is handled correctly by ndr_pull_advance.
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 6d05fb3ea772c3642624ec6e0fb4e8d099bcdb8e
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed Jan 22 14:16:02 2020 +1300
librpc ndr: NDR_PULL_ALIGN check for unsigned overflow
Handle uint32 overflow in NDR_PULL_ALIGN
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit 46edde8647810790141a685cea5d139c0784eab0
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri Jan 24 15:21:47 2020 +1300
librpc ndr tests: uint32 overflow in NDR_PULL_ALIGN
Check that uint32 overflow is handled correctly by NDR_NEED_BYTES.
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
commit ae6927e4f08dcea89729d8e54363e98effab6624
Author: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri Jan 24 10:41:35 2020 +1300
librpc ndr: Heap-buffer-overflow in lzxpress_decompress
Reproducer for oss-fuzz Issue 20083
Project: samba
Fuzzing Engine: libFuzzer
Fuzz Target: fuzz_ndr_drsuapi_TYPE_OUT
Job Type: libfuzzer_asan_samba
Platform Id: linux
Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x6040000002fd
Crash State:
lzxpress_decompress
ndr_pull_compression_xpress_chunk
ndr_pull_compression_start
Sanitizer: address (ASAN)
Recommended Security Severity: Medium
Credit to OSS-Fuzz
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
-----------------------------------------------------------------------
Summary of changes:
librpc/ndr/libndr.h | 12 +-
librpc/ndr/ndr.c | 6 +-
librpc/ndr/ndr_basic.c | 2 +-
librpc/tests/test_ndr.c | 144 +++++++++++++++++++++
librpc/wscript_build | 8 ++
python/samba/tests/blackbox/ndrdump.py | 13 ++
.../__init__.py => selftest/knownfail.d/bug-14236 | 0
source3/libsmb/nmblib.c | 3 +
.../tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt | 2 +-
source4/selftest/tests.py | 2 +
10 files changed, 184 insertions(+), 8 deletions(-)
create mode 100644 librpc/tests/test_ndr.c
copy buildtools/wafsamba/__init__.py => selftest/knownfail.d/bug-14236 (100%)
Changeset truncated at 500 lines:
diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h
index 58ef517d363..c2c7e263049 100644
--- a/librpc/ndr/libndr.h
+++ b/librpc/ndr/libndr.h
@@ -309,7 +309,10 @@ enum ndr_compression_alg {
} while (0)
#define NDR_PULL_NEED_BYTES(ndr, n) do { \
- if (unlikely((n) > ndr->data_size || ndr->offset + (n) > ndr->data_size)) { \
+ if (unlikely(\
+ (n) > ndr->data_size || \
+ ndr->offset + (n) > ndr->data_size || \
+ ndr->offset + (n) < ndr->offset)) { \
if (ndr->flags & LIBNDR_FLAG_INCOMPLETE_BUFFER) { \
uint32_t _available = ndr->data_size - ndr->offset; \
uint32_t _missing = n - _available; \
@@ -328,6 +331,13 @@ enum ndr_compression_alg {
if (unlikely(ndr->flags & LIBNDR_FLAG_PAD_CHECK)) { \
ndr_check_padding(ndr, n); \
} \
+ if(unlikely( \
+ ((ndr->offset + (n-1)) & (~(n-1))) < ndr->offset)) {\
+ return ndr_pull_error( \
+ ndr, \
+ NDR_ERR_BUFSIZE, \
+ "Pull align (overflow) %u", (unsigned)n); \
+ } \
ndr->offset = (ndr->offset + (n-1)) & ~(n-1); \
} \
if (unlikely(ndr->offset > ndr->data_size)) { \
diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c
index c772d53f6ed..f96a0bca08b 100644
--- a/librpc/ndr/ndr.c
+++ b/librpc/ndr/ndr.c
@@ -199,12 +199,8 @@ _PUBLIC_ enum ndr_err_code ndr_pull_pop(struct ndr_pull *ndr)
*/
_PUBLIC_ enum ndr_err_code ndr_pull_advance(struct ndr_pull *ndr, uint32_t size)
{
+ NDR_PULL_NEED_BYTES(ndr, size);
ndr->offset += size;
- if (ndr->offset > ndr->data_size) {
- return ndr_pull_error(ndr, NDR_ERR_BUFSIZE,
- "ndr_pull_advance by %u failed",
- size);
- }
return NDR_ERR_SUCCESS;
}
diff --git a/librpc/ndr/ndr_basic.c b/librpc/ndr/ndr_basic.c
index 0811a590971..693d9c86e3a 100644
--- a/librpc/ndr/ndr_basic.c
+++ b/librpc/ndr/ndr_basic.c
@@ -44,7 +44,7 @@ static void ndr_dump_data(struct ndr_print *ndr, const uint8_t *buf, int len);
_PUBLIC_ void ndr_check_padding(struct ndr_pull *ndr, size_t n)
{
size_t ofs2 = (ndr->offset + (n-1)) & ~(n-1);
- int i;
+ size_t i;
for (i=ndr->offset;i<ofs2;i++) {
if (ndr->data[i] != 0) {
break;
diff --git a/librpc/tests/test_ndr.c b/librpc/tests/test_ndr.c
new file mode 100644
index 00000000000..316c54368a0
--- /dev/null
+++ b/librpc/tests/test_ndr.c
@@ -0,0 +1,144 @@
+/*
+ * Tests for librpc ndr functions
+ *
+ * Copyright (C) Catalyst.NET Ltd 2020
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * from cmocka.c:
+ * These headers or their equivalents should be included prior to
+ * including
+ * this header file.
+ *
+ * #include <stdarg.h>
+ * #include <stddef.h>
+ * #include <setjmp.h>
+ *
+ * This allows test applications to use custom definitions of C standard
+ * library functions and types.
+ *
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "librpc/ndr/libndr.h"
+
+/*
+ * Test NDR_PULL_NEED_BYTES integer overflow handling.
+ */
+static enum ndr_err_code wrap_NDR_PULL_NEED_BYTES(
+ struct ndr_pull *ndr,
+ uint32_t bytes) {
+
+ NDR_PULL_NEED_BYTES(ndr, bytes);
+ return NDR_ERR_SUCCESS;
+}
+
+static void test_NDR_PULL_NEED_BYTES(void **state)
+{
+ struct ndr_pull ndr = {0};
+ enum ndr_err_code err;
+
+ ndr.data_size = UINT32_MAX;
+ ndr.offset = UINT32_MAX -1;
+
+ /*
+ * This will not cause an overflow
+ */
+ err = wrap_NDR_PULL_NEED_BYTES(&ndr, 1);
+ assert_int_equal(NDR_ERR_SUCCESS, err);
+
+ /*
+ * This will cause an overflow
+ * and (offset + n) will be less than data_size
+ */
+ err = wrap_NDR_PULL_NEED_BYTES(&ndr, 2);
+ assert_int_equal(NDR_ERR_BUFSIZE, err);
+}
+
+/*
+ * Test NDR_PULL_ALIGN integer overflow handling.
+ */
+static enum ndr_err_code wrap_NDR_PULL_ALIGN(
+ struct ndr_pull *ndr,
+ uint32_t bytes) {
+
+ NDR_PULL_ALIGN(ndr, bytes);
+ return NDR_ERR_SUCCESS;
+}
+
+static void test_NDR_PULL_ALIGN(void **state)
+{
+ struct ndr_pull ndr = {0};
+ enum ndr_err_code err;
+
+ ndr.data_size = UINT32_MAX;
+ ndr.offset = UINT32_MAX -1;
+
+ /*
+ * This will not cause an overflow
+ */
+ err = wrap_NDR_PULL_ALIGN(&ndr, 2);
+ assert_int_equal(NDR_ERR_SUCCESS, err);
+
+ /*
+ * This will cause an overflow
+ * and (offset + n) will be less than data_size
+ */
+ err = wrap_NDR_PULL_ALIGN(&ndr, 4);
+ assert_int_equal(NDR_ERR_BUFSIZE, err);
+}
+
+/*
+ * Test ndr_pull_advance integer overflow handling.
+ */
+static void test_ndr_pull_advance(void **state)
+{
+ struct ndr_pull ndr = {0};
+ enum ndr_err_code err;
+
+ ndr.data_size = UINT32_MAX;
+ ndr.offset = UINT32_MAX -1;
+
+ /*
+ * This will not cause an overflow
+ */
+ err = ndr_pull_advance(&ndr, 1);
+ assert_int_equal(NDR_ERR_SUCCESS, err);
+
+ /*
+ * This will cause an overflow
+ * and (offset + n) will be less than data_size
+ */
+ err = ndr_pull_advance(&ndr, 2);
+ assert_int_equal(NDR_ERR_BUFSIZE, err);
+}
+
+int main(int argc, const char **argv)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_NDR_PULL_NEED_BYTES),
+ cmocka_unit_test(test_NDR_PULL_ALIGN),
+ cmocka_unit_test(test_ndr_pull_advance),
+ };
+
+ cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 5eb78e6010a..ec8697fbcc5 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -698,3 +698,11 @@ bld.SAMBA_BINARY('test_ndr_string',
ndr
''',
for_selftest=True)
+
+bld.SAMBA_BINARY('test_ndr',
+ source='tests/test_ndr.c',
+ deps='''
+ cmocka
+ ndr
+ ''',
+ for_selftest=True)
diff --git a/python/samba/tests/blackbox/ndrdump.py b/python/samba/tests/blackbox/ndrdump.py
index b3c837819b1..205519c3f8a 100644
--- a/python/samba/tests/blackbox/ndrdump.py
+++ b/python/samba/tests/blackbox/ndrdump.py
@@ -437,3 +437,16 @@ dump OK
self.fail(e)
self.assertEqual(actual, expected)
+
+ def test_ndrdump_fuzzed_ndr_compression(self):
+ expected = 'pull returned Buffer Size Error'
+ command = (
+ "ndrdump drsuapi 3 out --base64-input "
+ "--input BwAAAAcAAAAGAAAAAwAgICAgICAJAAAAICAgIAkAAAAgIAAA//////8=")
+ try:
+ actual = self.check_exit_code(command, 2)
+ except BlackboxProcessError as e:
+ self.fail(e)
+ # check_output will return bytes
+ # convert expected to bytes for python 3
+ self.assertRegex(actual.decode('utf8'), expected + '$')
diff --git a/buildtools/wafsamba/__init__.py b/selftest/knownfail.d/bug-14236
similarity index 100%
copy from buildtools/wafsamba/__init__.py
copy to selftest/knownfail.d/bug-14236
diff --git a/source3/libsmb/nmblib.c b/source3/libsmb/nmblib.c
index 84cbb054b8e..c05fac2bba9 100644
--- a/source3/libsmb/nmblib.c
+++ b/source3/libsmb/nmblib.c
@@ -160,6 +160,9 @@ static bool handle_name_ptrs(unsigned char *ubuf,int *offset,int length,
if (!*got_pointer)
(*ret) += 2;
(*got_pointer)=True;
+ if (*offset > length - 2) {
+ return False;
+ }
(*offset) = ((ubuf[*offset] & ~0xC0)<<8) | ubuf[(*offset)+1];
if (loop_count++ == 10 ||
(*offset) < 0 || (*offset)>(length-2)) {
diff --git a/source4/librpc/tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt b/source4/librpc/tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt
index f489979d173..90879ad923e 100644
--- a/source4/librpc/tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt
+++ b/source4/librpc/tests/fuzzed_ntlmssp-CHALLENGE_MESSAGE.txt
@@ -38,6 +38,6 @@ pull returned Success
TargetInfoLen : 0x0000 (0)
TargetInfoMaxLen : 0x0000 (0)
TargetInfo : NULL
-ndr_push_subcontext_end: ndr_push_error(Subcontext Error): Bad subcontext (PUSH) content_size 1 is larger than size_is(0) at ../../librpc/ndr/ndr.c:905
+ndr_push_subcontext_end: ndr_push_error(Subcontext Error): Bad subcontext (PUSH) content_size 1 is larger than size_is(0) at ../../librpc/ndr/ndr.c:901
push returned Subcontext Error
validate push FAILED
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index f570d35dfba..ab2c4f69da0 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1334,6 +1334,8 @@ plantestsuite("libcli.drsuapi.repl_decrypt", "none",
[os.path.join(bindir(), "test_repl_decrypt")])
plantestsuite("librpc.ndr.ndr_string", "none",
[os.path.join(bindir(), "test_ndr_string")])
+plantestsuite("librpc.ndr.ndr", "none",
+ [os.path.join(bindir(), "test_ndr")])
# process restart and limit tests, these break the environment so need to run
# in their own specific environment
--
Samba Shared Repository
More information about the samba-cvs
mailing list