summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Hanselmann <public@hansmi.ch>2019-03-17 13:49:20 +0100
committerKarolin Seeger <kseeger@samba.org>2019-03-28 08:35:23 +0000
commitf3552ad511c8c2a343dd503c0faf3ea8410cf895 (patch)
tree2bf9a80d01ea747591ece5ce6edde9fc510555f3
parentb5ae06cc65322bc60c6dd1277c309db20d2ec2b2 (diff)
downloadsamba-f3552ad511c8c2a343dd503c0faf3ea8410cf895.tar.gz
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 <public@hansmi.ch> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> (cherry picked from commit 601afd690346087fbd53819dba9b1afa81560064)
-rw-r--r--source3/registry/regfio.c7
-rw-r--r--source3/registry/tests/test_regfio.c45
-rw-r--r--testdata/samba3/regfio_corrupt_hbin1.datbin0 -> 5120 bytes
-rw-r--r--testdata/samba3/regfio_corrupt_lf_subkeys.datbin0 -> 5120 bytes
4 files changed, 51 insertions, 1 deletions
diff --git a/source3/registry/regfio.c b/source3/registry/regfio.c
index ebc586c50be..33b24489e97 100644
--- a/source3/registry/regfio.c
+++ b/source3/registry/regfio.c
@@ -1132,6 +1132,10 @@ static bool next_record( REGF_HBIN *hbin, const char *hdr, bool *eob )
record_size = (record_size ^ 0xffffffff) + 1;
}
+ if ( record_size < sizeof(REC_HDR_SIZE) ) {
+ return False;
+ }
+
if ( memcmp( header, hdr, REC_HDR_SIZE ) == 0 ) {
found = True;
curr_off += sizeof(uint32_t);
@@ -1433,7 +1437,8 @@ 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 || !nk->subkeys.hashes || 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 */
diff --git a/source3/registry/tests/test_regfio.c b/source3/registry/tests/test_regfio.c
index ba557a34c98..228ce27d15a 100644
--- a/source3/registry/tests/test_regfio.c
+++ b/source3/registry/tests/test_regfio.c
@@ -95,6 +95,17 @@ static int teardown_context(void **state)
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 =
@@ -126,11 +137,45 @@ static void test_regfio_open_new_file(void **state)
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);
diff --git a/testdata/samba3/regfio_corrupt_hbin1.dat b/testdata/samba3/regfio_corrupt_hbin1.dat
new file mode 100644
index 00000000000..e74d6784239
--- /dev/null
+++ b/testdata/samba3/regfio_corrupt_hbin1.dat
Binary files 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
--- /dev/null
+++ b/testdata/samba3/regfio_corrupt_lf_subkeys.dat
Binary files differ