The branch, master has been updated via c0aca17a4c9 py/graph: use 2.6 compatible check for set membership via 68c0fc4335d regfio tests: Update comment style to match README.Coding via acbf103fcaa regfio: Update code near recent changes to match README.Coding via 601afd69034 regfio: Improve handling of malformed registry hive files via 9b2cb845b23 regfio: Add trivial unit test via aa6b355858a regfio: Use correct function names in debug information via 305346d360d Fix typos in "valid" from a40b0f452af build: Allow build when --disable-gnutls is set
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit c0aca17a4c9ec06f0127d5c972f3fa979a87a77f Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Mar 20 12:02:09 2019 +1300 py/graph: use 2.6 compatible check for set membership It is better this way anyway. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Wed Mar 20 06:36:05 UTC 2019 on sn-devel-144 commit 68c0fc4335d0c3c526a38481538a33290be6d58a Author: Andrew Bartlett <abart...@samba.org> Date: Wed Mar 20 17:33:46 2019 +1300 regfio tests: Update comment style to match README.Coding BUG: https://bugzilla.samba.org/show_bug.cgi?id=13840 Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit acbf103fcaa4150a57bfbab2450e36b5b39e399b Author: Andrew Bartlett <abart...@samba.org> Date: Wed Mar 20 17:32:39 2019 +1300 regfio: Update code near recent changes to match README.Coding This file long predates our current code conventions. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13840 Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit 601afd690346087fbd53819dba9b1afa81560064 Author: Michael Hanselmann <pub...@hansmi.ch> Date: Sun Mar 17 13:49:20 2019 +0100 regfio: Improve handling of malformed registry hive files * next_record: A malformed file can lead to an endless loop. * regfio_rootkey: Supplying a malformed registry hive file to the registry hive I/O code can lead to out-of-bounds reads. Test cases are included. Both issues resolved have been identified using AddressSanitizer. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13840 Signed-off-by: Michael Hanselmann <pub...@hansmi.ch> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9b2cb845b23cd1c91ab3b5ea8ad791b18b3ab733 Author: Michael Hanselmann <pub...@hansmi.ch> Date: Tue Mar 19 00:47:52 2019 +0100 regfio: Add trivial unit test An upcoming commit will resolve two cases of insufficient handling of mangled registry hive files and will include unit tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13840 Signed-off-by: Michael Hanselmann <pub...@hansmi.ch> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit aa6b355858a0d8b77bf49384e5329642add1a5ff Author: Michael Hanselmann <pub...@hansmi.ch> Date: Sun Mar 17 16:20:47 2019 +0100 regfio: Use correct function names in debug information Signed-off-by: Michael Hanselmann <pub...@hansmi.ch> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 305346d360d3c13fd315c1af27b037f46fd10650 Author: Michael Hanselmann <pub...@hansmi.ch> Date: Sun Mar 17 13:04:52 2019 +0100 Fix typos in "valid" s/vald/valid/ Signed-off-by: Michael Hanselmann <pub...@hansmi.ch> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/graph.py | 2 +- selftest/tests.py | 2 + source3/registry/regfio.c | 29 ++-- source3/registry/tests/test_regfio.c | 184 ++++++++++++++++++++++++++ source3/torture/torture.c | 2 +- source3/wscript_build | 6 + testdata/samba3/regfio_corrupt_hbin1.dat | Bin 0 -> 5120 bytes testdata/samba3/regfio_corrupt_lf_subkeys.dat | Bin 0 -> 5120 bytes 8 files changed, 214 insertions(+), 11 deletions(-) create mode 100644 source3/registry/tests/test_regfio.c create mode 100644 testdata/samba3/regfio_corrupt_hbin1.dat create mode 100644 testdata/samba3/regfio_corrupt_lf_subkeys.dat Changeset truncated at 500 lines: diff --git a/python/samba/graph.py b/python/samba/graph.py index 0a0aecd6631..c8d5f9230d0 100644 --- a/python/samba/graph.py +++ b/python/samba/graph.py @@ -88,7 +88,7 @@ def shorten_vertex_names(vertices, suffix=',...', aggressive=False): try: while True: c = set(x[i] for x in vlist) - if len(c) > 1 or c == {'*'}: + if len(c) > 1 or '*' in c: break i -= 1 except IndexError: diff --git a/selftest/tests.py b/selftest/tests.py index 9af4e5f327f..ac9ed934956 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -257,3 +257,5 @@ plantestsuite("samba.unittests.ms_fnmatch", "none", [os.path.join(bindir(), "default/lib/util/test_ms_fnmatch")]) plantestsuite("samba.unittests.ntlm_check", "none", [os.path.join(bindir(), "default/libcli/auth/test_ntlm_check")]) +plantestsuite("samba.unittests.test_registry_regfio", "none", + [os.path.join(bindir(), "default/source3/test_registry_regfio")]) diff --git a/source3/registry/regfio.c b/source3/registry/regfio.c index 9bb89ff11d4..32e166a1223 100644 --- a/source3/registry/regfio.c +++ b/source3/registry/regfio.c @@ -178,7 +178,7 @@ static int read_block( REGF_FILE *file, prs_struct *ps, uint32_t file_offset, ui return False; } if ( (returned == 0) && (bytes_read < block_size) ) { - DEBUG(0,("read_block: not a vald registry file ?\n" )); + DEBUG(0,("read_block: not a valid registry file ?\n" )); return False; } @@ -305,7 +305,7 @@ static bool prs_hbin_block( const char *desc, prs_struct *ps, int depth, REGF_HB { uint32_t block_size2; - prs_debug(ps, depth, desc, "prs_regf_block"); + prs_debug(ps, depth, desc, "prs_hbin_block"); depth++; if ( !prs_uint8s( True, "header", ps, depth, (uint8_t*)hbin->header, sizeof( hbin->header )) ) @@ -1019,7 +1019,7 @@ static bool hbin_prs_key( REGF_FILE *file, REGF_HBIN *hbin, REGF_NK_REC *nk ) int depth = 0; REGF_HBIN *sub_hbin; - prs_debug(&hbin->ps, depth, "", "fetch_key"); + prs_debug(&hbin->ps, depth, "", "prs_key"); depth++; /* get the initial nk record */ @@ -1127,12 +1127,16 @@ static bool next_record( REGF_HBIN *hbin, const char *hdr, bool *eob ) if ( !prs_uint8s( True, "header", ps, 0, header, REC_HDR_SIZE ) ) return False; - if ( record_size & 0x80000000 ) { + if (record_size & 0x80000000) { /* absolute_value(record_size) */ record_size = (record_size ^ 0xffffffff) + 1; } - if ( memcmp( header, hdr, REC_HDR_SIZE ) == 0 ) { + if (record_size < sizeof(REC_HDR_SIZE)) { + return false; + } + + if (memcmp(header, hdr, REC_HDR_SIZE) == 0) { found = True; curr_off += sizeof(uint32_t); } @@ -1238,7 +1242,7 @@ out: ZERO_STRUCTP( rb ); rb->fd = -1; - if ( !(rb->mem_ctx = talloc_init( "read_regf_block" )) ) { + if ( !(rb->mem_ctx = talloc_init( "regfio_open" )) ) { regfio_close( rb ); return NULL; } @@ -1433,12 +1437,19 @@ REGF_NK_REC* regfio_rootkey( REGF_FILE *file ) /* see if there is anything left to report */ - if ( !nk || (nk->subkeys_off==REGF_OFFSET_NONE) || (nk->subkey_index >= nk->num_subkeys) ) + if (nk == NULL || + nk->subkeys.hashes == NULL || + nk->subkey_index >= nk->subkeys.num_keys || + (nk->subkeys_off == REGF_OFFSET_NONE) || + (nk->subkey_index >= nk->num_subkeys)) { return NULL; + } /* find the HBIN block which should contain the nk record */ - - if ( !(hbin = lookup_hbin_block( file, nk->subkeys.hashes[nk->subkey_index].nk_off )) ) { + + hbin = lookup_hbin_block(file, + nk->subkeys.hashes[nk->subkey_index].nk_off); + if (hbin == NULL) { DEBUG(0,("hbin_prs_key: Failed to find HBIN block containing offset [0x%x]\n", nk->subkeys.hashes[nk->subkey_index].nk_off)); return NULL; diff --git a/source3/registry/tests/test_regfio.c b/source3/registry/tests/test_regfio.c new file mode 100644 index 00000000000..86a217661f3 --- /dev/null +++ b/source3/registry/tests/test_regfio.c @@ -0,0 +1,184 @@ +/* + * Unix SMB/CIFS implementation. + * + * Copyright (C) 2019 Michael Hanselmann <pub...@hansmi.ch> + * + * 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/>. + */ + +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include <cmocka.h> + +#include <errno.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> + +#include "includes.h" +#include "lib/replace/replace.h" +#include "lib/util/samba_util.h" +#include "registry/regfio.h" + +struct test_ctx { + char *tmp_regfile; + int tmp_regfile_fd; + REGF_FILE *rb; +}; + +static int setup_context(void **state) +{ + struct test_ctx *test_ctx; + + test_ctx = talloc_zero(NULL, struct test_ctx); + assert_non_null(test_ctx); + + test_ctx->tmp_regfile_fd = -1; + + *state = test_ctx; + + return 0; +} + +static int setup_context_tempfile(void **state) +{ + struct test_ctx *test_ctx; + int ret; + + ret = setup_context(state); + + if (ret == 0) { + test_ctx = talloc_get_type_abort(*state, struct test_ctx); + + test_ctx->tmp_regfile = talloc_strdup(test_ctx, "/tmp/regfio.XXXXXX"); + assert_non_null(test_ctx->tmp_regfile); + + test_ctx->tmp_regfile_fd = mkstemp(test_ctx->tmp_regfile); + assert_return_code(test_ctx->tmp_regfile_fd, errno); + } + + return ret; +} + +static int teardown_context(void **state) +{ + struct test_ctx *test_ctx = + talloc_get_type_abort(*state, struct test_ctx); + + if (test_ctx->rb) { + regfio_close(test_ctx->rb); + } + + if (test_ctx->tmp_regfile) { + unlink(test_ctx->tmp_regfile); + } + + if (test_ctx->tmp_regfile_fd != -1) { + close(test_ctx->tmp_regfile_fd); + } + + talloc_free(test_ctx); + + return 0; +} + +static void open_testfile(struct test_ctx *test_ctx, const char *filename) +{ + char *path; + + path = talloc_asprintf(test_ctx, "%s/testdata/samba3/%s", SRCDIR, filename); + assert_non_null(path); + + test_ctx->rb = regfio_open(path, O_RDONLY, 0600); + assert_non_null(test_ctx->rb); +} + +static void test_regfio_open_new_file(void **state) +{ + struct test_ctx *test_ctx = + talloc_get_type_abort(*state, struct test_ctx); + REGF_NK_REC *root; + struct regval_ctr *values; + struct regsubkey_ctr *subkeys; + WERROR werr; + + test_ctx->rb = regfio_open(test_ctx->tmp_regfile, + O_RDWR | O_CREAT | O_TRUNC, 0600); + assert_non_null(test_ctx->rb); + + root = regfio_rootkey(test_ctx->rb); + assert_null(root); + + werr = regsubkey_ctr_init(NULL, &subkeys); + assert_true(W_ERROR_IS_OK(werr)); + + werr = regval_ctr_init(subkeys, &values); + assert_true(W_ERROR_IS_OK(werr)); + + /* Write root key */ + regfio_write_key(test_ctx->rb, "", values, subkeys, NULL, NULL); + + root = regfio_rootkey(test_ctx->rb); + assert_non_null(root); + assert_memory_equal(root->header, "nk", sizeof(root->header)); + assert_int_equal(root->key_type, NK_TYPE_ROOTKEY); +} + +static void test_regfio_corrupt_hbin(void **state) +{ + struct test_ctx *test_ctx = + talloc_get_type_abort(*state, struct test_ctx); + REGF_NK_REC *root; + + open_testfile(test_ctx, "regfio_corrupt_hbin1.dat"); + + root = regfio_rootkey(test_ctx->rb); + assert_null(root); +} + +static void test_regfio_corrupt_lf_subkeys(void **state) +{ + struct test_ctx *test_ctx = + talloc_get_type_abort(*state, struct test_ctx); + REGF_NK_REC *root, *subkey; + + open_testfile(test_ctx, "regfio_corrupt_lf_subkeys.dat"); + + root = regfio_rootkey(test_ctx->rb); + assert_non_null(root); + + root->subkey_index = 0; + while ((subkey = regfio_fetch_subkey(test_ctx->rb, root))) { + } +} + +int main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_regfio_open_new_file, + setup_context_tempfile, + teardown_context), + cmocka_unit_test_setup_teardown(test_regfio_corrupt_hbin, + setup_context, + teardown_context), + cmocka_unit_test_setup_teardown(test_regfio_corrupt_lf_subkeys, + setup_context, + teardown_context), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 8b1bf326769..639ab27e3b9 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -9653,7 +9653,7 @@ static bool run_uid_regression_test(int dummy) goto out; } - /* Now try a SMBtdis with the invald vuid set to zero. */ + /* Now try a SMBtdis with the invalid vuid set to zero. */ cli_state_set_uid(cli, 0); /* This should succeed. */ diff --git a/source3/wscript_build b/source3/wscript_build index ed4de978fdc..e633c7046ca 100644 --- a/source3/wscript_build +++ b/source3/wscript_build @@ -203,6 +203,12 @@ bld.SAMBA3_SUBSYSTEM('REGFIO', source='registry/regfio.c', deps='samba-util REG_PARSE_PRS') +bld.SAMBA_BINARY('test_registry_regfio', + source='registry/tests/test_regfio.c', + deps='cmocka samba3-util smbconf REGFIO', + local_include=False, + install=False) + bld.SAMBA3_SUBSYSTEM('REG_API_REGF', source='registry/reg_api_regf.c', deps='samba-util') diff --git a/testdata/samba3/regfio_corrupt_hbin1.dat b/testdata/samba3/regfio_corrupt_hbin1.dat new file mode 100644 index 00000000000..e74d6784239 Binary files /dev/null and b/testdata/samba3/regfio_corrupt_hbin1.dat differ diff --git a/testdata/samba3/regfio_corrupt_lf_subkeys.dat b/testdata/samba3/regfio_corrupt_lf_subkeys.dat new file mode 100644 index 00000000000..c540051f7f1 Binary files /dev/null and b/testdata/samba3/regfio_corrupt_lf_subkeys.dat differ -- Samba Shared Repository