Skip to content

Commit

Permalink
oLschema2ldif: Resolve multiple parsing bugs
Browse files Browse the repository at this point in the history
The "oLschema2ldif" program contained multiple bugs triggered by
malformed inputs:

* Iteration beyond list of recognized dsdb syntax OIDs when value wasn't
  found (bug 9567)
* NULL pointer dereference when input didn't define a name
* Heap buffer overflows for unterminated token values

Tests are added to reproduce all identified bugs.

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

Signed-off-by: Michael Hanselmann <[email protected]>
Reviewed-by: Andrew Bartlett <[email protected]>
Reviewed-by: Douglas Bagnall <[email protected]>

Then adapted to use ARRAY_SIZE() consistently as suggested by
metze.

Autobuild-User(master): Andrew Bartlett <[email protected]>
Autobuild-Date(master): Wed Apr  3 02:43:07 UTC 2019 on sn-devel-144
  • Loading branch information
hansmi authored and abartlet committed Apr 3, 2019
1 parent 4ae2fb2 commit 29d7c80
Show file tree
Hide file tree
Showing 5 changed files with 240 additions and 4 deletions.
2 changes: 2 additions & 0 deletions selftest/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,3 +258,5 @@ def cmdline(script, *args):
[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")])
plantestsuite("samba.unittests.test_oLschema2ldif", "none",
[os.path.join(bindir(), "default/source4/utils/oLschema2ldif/test_oLschema2ldif")])
6 changes: 3 additions & 3 deletions source4/dsdb/schema/schema_syntax.c
Original file line number Diff line number Diff line change
Expand Up @@ -2640,7 +2640,7 @@ static const struct dsdb_syntax dsdb_syntaxes[] = {
const struct dsdb_syntax *find_syntax_map_by_ad_oid(const char *ad_oid)
{
unsigned int i;
for (i=0; dsdb_syntaxes[i].ldap_oid; i++) {
for (i=0; i < ARRAY_SIZE(dsdb_syntaxes); i++) {
if (strcasecmp(ad_oid, dsdb_syntaxes[i].attributeSyntax_oid) == 0) {
return &dsdb_syntaxes[i];
}
Expand All @@ -2651,7 +2651,7 @@ const struct dsdb_syntax *find_syntax_map_by_ad_oid(const char *ad_oid)
const struct dsdb_syntax *find_syntax_map_by_ad_syntax(int oMSyntax)
{
unsigned int i;
for (i=0; dsdb_syntaxes[i].ldap_oid; i++) {
for (i=0; i < ARRAY_SIZE(dsdb_syntaxes); i++) {
if (oMSyntax == dsdb_syntaxes[i].oMSyntax) {
return &dsdb_syntaxes[i];
}
Expand All @@ -2662,7 +2662,7 @@ const struct dsdb_syntax *find_syntax_map_by_ad_syntax(int oMSyntax)
const struct dsdb_syntax *find_syntax_map_by_standard_oid(const char *standard_oid)
{
unsigned int i;
for (i=0; dsdb_syntaxes[i].ldap_oid; i++) {
for (i=0; i < ARRAY_SIZE(dsdb_syntaxes); i++) {
if (strcasecmp(standard_oid, dsdb_syntaxes[i].ldap_oid) == 0) {
return &dsdb_syntaxes[i];
}
Expand Down
23 changes: 22 additions & 1 deletion source4/utils/oLschema2ldif/lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ static char *get_def_value(TALLOC_CTX *ctx, char **string)
n = strcspn(c, "\'");
value = talloc_strndup(ctx, c, n);
c += n;
c++; /* skip closing \' */
if (*c != '\0') {
c++; /* skip closing \' */
}
} else {
n = strcspn(c, " \t\n");
value = talloc_strndup(ctx, c, n);
Expand Down Expand Up @@ -177,6 +179,10 @@ static struct schema_token *get_next_schema_token(TALLOC_CTX *ctx, char **string
n = strcspn(c, ")");
token->value = talloc_strndup(ctx, c, n);
c += n;
if (*c == '\0') {
talloc_free(token->value);
return NULL;
}
c++;
} else {
token->value = get_def_value(ctx, &c);
Expand Down Expand Up @@ -217,6 +223,10 @@ static struct schema_token *get_next_schema_token(TALLOC_CTX *ctx, char **string
n = strcspn(c, ")");
token->value = talloc_strndup(ctx, c, n);
c += n;
if (*c == '\0') {
talloc_free(token->value);
return NULL;
}
c++;
} else {
token->value = get_def_value(ctx, &c);
Expand All @@ -236,6 +246,10 @@ static struct schema_token *get_next_schema_token(TALLOC_CTX *ctx, char **string
n = strcspn(c, ")");
token->value = talloc_strndup(ctx, c, n);
c += n;
if (*c == '\0') {
talloc_free(token->value);
return NULL;
}
c++;
} else {
token->value = get_def_value(ctx, &c);
Expand Down Expand Up @@ -316,6 +330,9 @@ static struct schema_token *get_next_schema_token(TALLOC_CTX *ctx, char **string
}
if (*c == '\'') {
c = strchr(++c, '\'');
if (c == NULL || *c == '\0') {
return NULL;
}
c++;
} else {
c += strcspn(c, " \t\n");
Expand Down Expand Up @@ -486,12 +503,16 @@ static struct ldb_message *process_entry(TALLOC_CTX *mem_ctx, struct conv_option

default:
fprintf(stderr, "Unknown Definition: %s\n", token->value);
goto failed;
}
}

if (isAttribute) {
MSG_ADD_STRING("isSingleValued", single_valued ? "TRUE" : "FALSE");
} else {
if (msg->dn == NULL) {
goto failed;
}
MSG_ADD_STRING("defaultObjectCategory", ldb_dn_get_linearized(msg->dn));
}

Expand Down
206 changes: 206 additions & 0 deletions source4/utils/oLschema2ldif/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
/*
* Unix SMB/CIFS implementation.
*
* Copyright (C) 2019 Michael Hanselmann <[email protected]>
*
* 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 "includes.h"
#include "./lib.h"

struct test_ctx {
};

static int setup_context(void **state)
{
struct test_ctx *test_ctx;

test_ctx = talloc_zero(NULL, struct test_ctx);
assert_non_null(test_ctx);

*state = test_ctx;

return 0;
}

static int teardown_context(void **state)
{
struct test_ctx *test_ctx =
talloc_get_type_abort(*state, struct test_ctx);

talloc_free(test_ctx);

return 0;
}

static struct schema_conv process_data_blob(void **state, DATA_BLOB input)
{
struct test_ctx *test_ctx =
talloc_get_type_abort(*state, struct test_ctx);
struct conv_options opt;
struct schema_conv ret;

assert_non_null(test_ctx);
assert_non_null(input.data);

opt.in = fmemopen(input.data, input.length, "r");
opt.out = fopen("/dev/null", "w");
opt.ldb_ctx = ldb_init(test_ctx, NULL);

assert_non_null(opt.in);
assert_non_null(opt.out);
assert_non_null(opt.ldb_ctx);

opt.basedn = ldb_dn_new(test_ctx, opt.ldb_ctx, "");

assert_non_null(opt.basedn);

ret = process_file(test_ctx, &opt);

fclose(opt.in);
fclose(opt.out);

return ret;
}

static void test_unknown_syntax_oid(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"attributetype ( 999.555.999.555.999\n"
"NAME 'mailLocalAddress'\n"
"DESC 'RFC822 email address of this recipient'\n"
"EQUALITY caseIgnoreIA5Match\n"
"SYNTAX 999.555.999.555.999{256} )\n"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

static void test_unterminated_token_value(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"attributetype ( 2.16.840.1.113730.3.1.47\n"
"\tNAME 'mailRoutingAX 1.3.6.1.4.1.1466.115.121.1.26{256}\n"
"\tSI GLE-VALUE )\n"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

static void test_unterminated_must_value(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"attributetype ( 1\n"
"\tSYNTAX 1./)# MUST ( foobar $\n"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

static void test_unterminated_may_value(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"attributetype ( 1\n"
"\tSYNTAX 1.3.6.1.4.1.1466.115.121.1./)# MAY ( javaClassNames $\n"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

static void test_unterminated_sup_value(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"attributetype ( 1\n"
"\tSYNTAX 1./)# SUP ( foobar $\n"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

static void test_unknown_token(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"attributetype ( 1\n"
"\tFOOBAR 123\n"
" )\n"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

static void test_missing_name(void **state)
{
struct schema_conv ret;

ret = process_data_blob(state, data_blob_string_const(
"objectclass ( 1.3.6.3.6.1.4.1.1466.115.121.1.26{256} )"
));

assert_int_equal(ret.count, 1);
assert_int_equal(ret.failures, 1);
}

int main(void) {
const struct CMUnitTest tests[] = {
cmocka_unit_test_setup_teardown(test_unknown_syntax_oid,
setup_context,
teardown_context),
cmocka_unit_test_setup_teardown(test_unterminated_token_value,
setup_context,
teardown_context),
cmocka_unit_test_setup_teardown(test_unterminated_must_value,
setup_context,
teardown_context),
cmocka_unit_test_setup_teardown(test_unterminated_may_value,
setup_context,
teardown_context),
cmocka_unit_test_setup_teardown(test_unterminated_sup_value,
setup_context,
teardown_context),
cmocka_unit_test_setup_teardown(test_unknown_token,
setup_context,
teardown_context),
cmocka_unit_test_setup_teardown(test_missing_name,
setup_context,
teardown_context),
};

cmocka_set_message_output(CM_OUTPUT_SUBUNIT);

return cmocka_run_group_tests(tests, NULL, NULL);
}
7 changes: 7 additions & 0 deletions source4/utils/oLschema2ldif/wscript_build
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,10 @@ bld.SAMBA_BINARY('oLschema2ldif',
manpages='oLschema2ldif.1',
deps='oLschema2ldif-lib POPT_SAMBA',
)

bld.SAMBA_BINARY('test_oLschema2ldif',
source='test.c',
deps='cmocka oLschema2ldif-lib',
local_include=False,
install=False,
)

0 comments on commit 29d7c80

Please sign in to comment.