From: Gary Lockyer Date: Thu, 23 Jan 2020 21:41:35 +0000 (+1300) Subject: librpc ndr: Heap-buffer-overflow in lzxpress_decompress X-Git-Tag: ldb-2.1.1~153 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ae6927e4f08dcea89729d8e54363e98effab6624;p=thirdparty%2Fsamba.git 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 Reviewed-by: Andrew Bartlett --- diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h index 58ef517d363..b7cccf3dfc5 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; \ diff --git a/librpc/tests/test_ndr.c b/librpc/tests/test_ndr.c new file mode 100644 index 00000000000..1c074d71023 --- /dev/null +++ b/librpc/tests/test_ndr.c @@ -0,0 +1,84 @@ +/* + * 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 . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ +#include +#include +#include +#include +#include + +#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); +} + +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_NDR_PULL_NEED_BYTES), + }; + + 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/selftest/knownfail.d/bug-14236 b/selftest/knownfail.d/bug-14236 new file mode 100644 index 00000000000..64b956997a6 --- /dev/null +++ b/selftest/knownfail.d/bug-14236 @@ -0,0 +1 @@ +^samba.tests.blackbox.ndrdump.samba.tests.blackbox.ndrdump.NdrDumpTests.test_ndrdump_fuzzed_ndr_compression 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