ldb: Avoid read beyond buffer

Calling the "ldb_parse_tree" function with a filter consisting of
exactly a single space (" ") would trigger a read beyond the input
buffer. A unittest is included.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13900

Signed-off-by: Michael Hanselmann <public@hansmi.ch>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Tim Beale <timbeale@catalyst.net.nz>

diff --git a/lib/ldb/common/ldb_parse.c b/lib/ldb/common/ldb_parse.c
index db4200913119c2557e98ea1d11ba56a028b3ce29..452c5830ed59732236b90493de1cdf36ef4a646b 100644 (file)
--- a/lib/ldb/common/ldb_parse.c
+++ b/lib/ldb/common/ldb_parse.c
@@ -328,7 +328,7 @@ static enum ldb_parse_op ldb_parse_filtertype(TALLOC_CTX *mem_ctx, char **type,
 
        if (*p == '=') {
                filter = LDB_OP_EQUALITY;
-       } else if (*(p + 1) == '=') {
+       } else if (*p != '\0' && *(p + 1) == '=') {
                switch (*p) {
                case '<':
                        filter = LDB_OP_LESS;
@@ -679,12 +679,12 @@ static struct ldb_parse_tree *ldb_parse_filter(TALLOC_CTX *mem_ctx, const char *
 */
 struct ldb_parse_tree *ldb_parse_tree(TALLOC_CTX *mem_ctx, const char *s)
 {
+       while (s && isspace((unsigned char)*s)) s++;
+
        if (s == NULL || *s == 0) {
                s = "(|(objectClass=*)(distinguishedName=*))";
        }
 
-       while (isspace((unsigned char)*s)) s++;
-
        if (*s == '(') {
                return ldb_parse_filter(mem_ctx, &s);
        }

diff --git a/lib/ldb/tests/ldb_parse_test.c b/lib/ldb/tests/ldb_parse_test.c
new file mode 100644 (file)
index 0000000..a739d77
--- /dev/null
+++ b/lib/ldb/tests/ldb_parse_test.c
@@ -0,0 +1,93 @@
+/*
+ * Tests exercising the ldb parse operations.
+ *
+ * Copyright (C) Catalyst.NET Ltd 2017
+ * Copyright (C) Michael Hanselmann 2019
+ *
+ * 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 <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "../include/ldb.h"
+
+struct test_ctx
+{
+};
+
+static int setup(void **state)
+{
+       struct test_ctx *ctx;
+
+       ctx = talloc_zero(NULL, struct test_ctx);
+       assert_non_null(ctx);
+
+       *state = ctx;
+
+       return 0;
+}
+
+static int teardown(void **state)
+{
+       struct test_ctx *ctx =
+               talloc_get_type_abort(*state, struct test_ctx);
+
+       talloc_free(ctx);
+
+       return 0;
+}
+
+static void test_roundtrip(TALLOC_CTX *mem_ctx, const char *filter, const char *expected)
+{
+       struct ldb_parse_tree *tree;
+       char *serialized;
+
+       assert_non_null(filter);
+       assert_non_null(expected);
+
+       tree = ldb_parse_tree(mem_ctx, filter);
+       assert_non_null(tree);
+
+       serialized = ldb_filter_from_tree(mem_ctx, tree);
+       assert_non_null(serialized);
+
+       assert_string_equal(serialized, expected);
+}
+
+static void test_parse_filtertype(void **state)
+{
+       struct test_ctx *ctx =
+               talloc_get_type_abort(*state, struct test_ctx);
+
+       test_roundtrip(ctx, "", "(|(objectClass=*)(distinguishedName=*))");
+       test_roundtrip(ctx, "a=value", "(a=value)");
+       test_roundtrip(ctx, "(|(foo=bar)(baz=hello))", "(|(foo=bar)(baz=hello))");
+       test_roundtrip(ctx, " ", "(|(objectClass=*)(distinguishedName=*))");
+}
+
+int main(int argc, const char **argv)
+{
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test_setup_teardown(test_parse_filtertype, setup, teardown),
+       };
+
+       cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}

diff --git a/lib/ldb/wscript b/lib/ldb/wscript
index 8b22e9b2c7a8de0af5a3be06a71f15eda42dda12..52307e2321e2931888ef61fbd27b40d55a1b4d71 100644 (file)
--- a/lib/ldb/wscript
+++ b/lib/ldb/wscript
@@ -506,6 +506,11 @@ def build(bld):
                          deps='cmocka ldb ldb_tdb_err_map',
                          install=False)
 
+        bld.SAMBA_BINARY('ldb_parse_test',
+                         source='tests/ldb_parse_test.c',
+                         deps='cmocka ldb ldb_tdb_err_map',
+                         install=False)
+
         if bld.CONFIG_SET('HAVE_LMDB'):
             bld.SAMBA_BINARY('ldb_mdb_mod_op_test',
                              source='tests/ldb_mod_op_test.c',
@@ -576,7 +581,8 @@ def test(ctx):
                  'ldb_tdb_kv_ops_test',
                  'ldb_tdb_test',
                  'ldb_match_test',
-                 'ldb_key_value_test']
+                 'ldb_key_value_test',
+                 'ldb_parse_test']
 
     if env.HAVE_LMDB:
         test_exes += ['ldb_mdb_mod_op_test',