1
0
mirror of https://github.com/samba-team/samba.git synced 2025-01-21 18:04:06 +03:00

47 Commits

Author SHA1 Message Date
Douglas Bagnall
73107aa9f6 ldb: fix Coverity 1636883
oops.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Martin Schwenke <martin@meltin.net>

Autobuild-User(master): Martin Schwenke <martins@samba.org>
Autobuild-Date(master): Mon Dec 30 04:17:46 UTC 2024 on atb-devel-224
2024-12-30 04:17:46 +00:00
Douglas Bagnall
d8b7712c53 ldb:dn_compare_base: avoid unlikely int overflow
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-12-19 23:00:32 +00:00
Douglas Bagnall
4fa67dee9a ldb:dn_compare: be a bit more transitive
If neither dn can casefold, they should be considered equal. Otherwise
cmp(dn1, dn2) will be inconsistent with cmp(dn2, dn1).

These will still sort to the end of the list, relative to any valid
DNs.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-12-19 23:00:32 +00:00
Douglas Bagnall
df3f7be326 ldb:dn_casefold_internal: TALLOC_FREE only what we talloced
If the failure is not on the last component, we would have
TALLOC_FREE()ed some components that we hadn't set.

I think in all pathways we initialise the unset components to zero,
but we should be careful just in case.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2024-12-19 23:00:32 +00:00
Volker Lendecke
542cf01bfe ldb: User hexchars_upper from replace.h
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2024-11-12 12:09:35 +00:00
Jo Sutton
4e8ca6140a ldb: Attach appropriate ldb context to returned result
This is done by adding a new API that avoids the problems of
ldb_dn_copy() and makes it clear that a struct ldb_context *
pointer will be stored in the new copy.

Signed-off-by: Jo Sutton <josutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-06-11 04:32:30 +00:00
Douglas Bagnall
af7654331f ldb: avoid NULL deref in ldb_db_compare
This also sorts NULLs after invalid DNs, which matches the comment
above.

CID 1596622.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-05-07 23:25:35 +00:00
Douglas Bagnall
a9eaf8a3ab ldb: comment for ldb_dn_compare_base
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-23 01:33:29 +00:00
Douglas Bagnall
5fe488d515 ldb:dn: make ldb_dn_compare() self-consistent
We were returning -1 in all these cases:

   ldb_dn_compare(dn, NULL);
   ldb_dn_compare(NULL, dn);
   ldb_dn_compare(NULL, NULL);

which would give strange results in sort, where this is often used.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-23 01:33:29 +00:00
Douglas Bagnall
75e51bd99b ldb:ldb_dn: use safe NUMERIC_CMP in ldb_dn_compare()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
5150b318f4 ldb:ldb_dn: use safe NUMERIC_CMP in ldb_dn_compare_base()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
d13226366b ldb_dn: make LDB_FREE, TALLOC_FREE
This LDB_FREE() seems to predate TALLOC_FREE(), and was identical
until TALLOC_FREE was optimised to avoid calling talloc_free(NULL) in
b9fcfc6399eab750880ee0b9806311dd351a8ff6.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-03-20 23:42:34 +00:00
Douglas Bagnall
8cf77b5775 ldb:ldb_dn: use safe transitive comparison in ldb_dn_compare()
The comparison we make is unconventional, and makes no difference in
normal usage, where we just want to know whether two DNs are the same
or not. But with over 100 callers, it is possible that something
somewhere is attempting a sort.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-03-20 23:42:34 +00:00
Volker Lendecke
300ad4ff12 lib: Save intermediate NULL checks with talloc_asprintf_addbuf()
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2022-12-14 04:32:34 +00:00
Volker Lendecke
9aca11a71a ldb: Fix a typo
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2022-09-07 18:40:28 +00:00
Volker Lendecke
ef846e660a ldb: Avoid "==true/false" in a boolean expression
That's what we have boolean variables and expressions for

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2022-04-26 21:41:29 +00:00
Douglas Bagnall
dbb3e65f7e CVE-2020-27840 ldb_dn: avoid head corruption in ldb_dn_explode
A DN string with lots of trailing space can cause ldb_dn_explode() to
put a zero byte in the wrong place in the heap.

When a DN string has a value represented with trailing spaces,
like this

     "CN=foo   ,DC=bar"

the whitespace is supposed to be ignored. We keep track of this in the
`t` pointer, which is NULL when we are not walking through trailing
spaces, and points to the first space when we are. We are walking with
the `p` pointer, writing the value to `d`, and keeping the length in
`l`.

     "CN=foo   ,DC= "       ==>       "foo   "
            ^  ^                             ^
            t  p                             d
                                       --l---

The value is finished when we encounter a comma or the end of the
string. If `t` is not NULL at that point, we assume there are trailing
spaces and wind `d and `l` back by the correct amount. Then we switch
to expecting an attribute name (e.g. "CN"), until we get to an "=",
which puts us back into looking for a value.

Unfortunately, we forget to immediately tell `t` that we'd finished
the last value, we can end up like this:

     "CN=foo   ,DC= "       ==>        ""
            ^      ^                    ^
            t      p                    d
                                        l=0

where `p` is pointing to a new value that contains only spaces, while
`t` is still referring to the old value. `p` notices the value ends,
and we subtract `p - t` from `d`:

     "CN=foo   ,DC= "       ==>  ?     ""
            ^       ^            ^
            t       p            d
                                      l ~= SIZE_MAX - 8

At that point `d` wants to terminate its string with a '\0', but
instead it terminates someone else's byte. This does not crash if the
number of trailing spaces is small, as `d` will point into a previous
value (a copy of "foo" in this example). Corrupting that value will
ultimately not matter, as we will soon try to allocate a buffer `l`
long, which will be greater than the available memory and the whole
operation will fail properly.

However, with more spaces, `d` will point into memory before the
beginning of the allocated buffer, with the exact offset depending on
the length of the earlier attributes and the number of spaces.

What about a longer DN with more attributes? For example,
"CN=foo     ,DC= ,DC=example,DC=com" -- since `d` has moved out of
bounds, won't we continue to use it and write more DN values into
mystery memory? Fortunately not, because the aforementioned allocation
of `l` bytes must happen first, and `l` is now huge. The allocation
happens in a talloc_memdup(), which is by default restricted to
allocating 256MB.

So this allows a person who controls a string parsed by ldb_dn_explode
to corrupt heap memory by placing a single zero byte at a chosen
offset before the allocated buffer.

An LDAP bind request can send a string DN as a username. This DN is
necessarily parsed before the password is checked, so an attacker does
not need proper credentials. The attacker can easily cause a denial of
service and we cannot rule out more subtle attacks.

The immediate solution is to reset `t` to NULL when a comma is
encountered, indicating that we are no longer looking at trailing
whitespace.

Found with the help of Honggfuzz.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-03-24 12:05:32 +00:00
Volker Lendecke
130502af0b ldb: Use ARRAY_DEL_ELEMENT() in ldb_dn_set_extended_component()
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2020-03-26 14:43:32 +00:00
Volker Lendecke
f2a4eecbb3 ldb: Use ARRAY_DEL_ELEMENT() in ldb_dn_extended_filter()
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2020-03-26 14:43:32 +00:00
Andrew Bartlett
a8a3cef3a7 ldb: Do not read beyond the end of the extended DN component when printing
The print functions used in Samba NULL terminate, but do not assume they will

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-08-27 04:44:41 +00:00
Andrew Bartlett
52bd2dde5a ldb: Add test with == true or false to boolean if statements in ldb_dn_explode()
This is beyond the normal level of clarity we expect in Samba, and is of course
rudundent, but this is a complex routine that has confusing tests, some of
pointers and some of boolean state values.

This tries to make the code as clear as possible pending a more comprehensive
rewrite.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-08-27 04:44:41 +00:00
Andrew Bartlett
3f290e95c2 ldb: Rework all pointer NULL tests to use Samba's normal style
Also avoid if () without braces

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-08-27 04:44:41 +00:00
Douglas Bagnall
54f30f2fe3 ldb: don't try to save a value that isn't there
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14049

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-08-27 04:44:40 +00:00
Swen Schillig
99b4791cfe ldb: Fix mem-leak if talloc_realloc fails
In case of a failing talloc_realloc(), the only reference
to the originally allocated memory is overwritten.
Instead use a temp var until success is verified.

Signed-off-by: Swen Schillig <swen@linux.ibm.com>
Reviewed-by: Christof Schmitt <cs@samba.org>
Reviewed-by: Matthias Dieter Wallnöfer <mdw@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2019-08-10 19:24:29 +00:00
Douglas Bagnall
b136f153b8 ldb_dn: free dn components on explode failure
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Noel Power <npower@samba.org>
2019-08-06 17:00:38 +00:00
Douglas Bagnall
19a13cbe06 ldb: do not allow adding a DN as a base to itself
If you try to add a dn to itself, it expands as it goes. The resulting
loop cannot end well.

It looks like this in Python:

    dn = ldb.Dn(ldb.Ldb(), 'CN=y,DC=x')
    dn.add_base(dn)

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-07-10 04:32:13 +00:00
Douglas Bagnall
aa18f62a8a ldb: avoid NULL deref in ldb_dn_from_ldb_val (CID 1034730)
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-05-09 22:39:27 +00:00
Douglas Bagnall
d21801b888 ldb_dn: don't free a known NULL pointer
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2019-02-13 04:15:15 +01:00
Douglas Bagnall
d4ebe00688 ldb_dn: remove unreachable code in dn_explode
Every time I look at this file, I spend a few minutes wondering how
these bits of code are ever run. Never again.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2019-02-13 04:15:14 +01:00
Andrew Bartlett
2dafbd3213 ldb: Add new function ldb_dn_add_child_val()
This is safer for untrusted input than ldb_dn_add_child_fmt()

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2018-08-15 07:08:24 +02:00
Andreas Schneider
2b01bd0475 lib:ldb: Add FALL_THROUGH statements in common/ldb_dn.c
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2018-03-01 04:37:41 +01:00
Volker Lendecke
a320f53cb7 ldb: Fix two signed/unsigned hickups
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2016-08-05 22:20:05 +02:00
Andrew Bartlett
4fb23630ba ldb: Do not allocate the extended DN name
The name must be a hard-coded value from struct ldb_dn_extended_syntax
so just point to that constant pointer

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2016-06-27 00:18:17 +02:00
Petr Cech
32b1f78b23 LDB: Redudant test on NULL context remove
There is redudant test on NULL context in ldb_dn_new_fmt() function.
We use this (NULL) context in talloc_vasprintf() function which is
able to work with NULL at all. And at the end, we free this newly
created (by talloc_vasprintf) context. So it should be safe to remove
this check.

Signed-off-by: Petr Cech <pcech@redhat.com>
Reviewed-by: Simo Sorce <idra@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Tue Apr 26 08:09:25 CEST 2016 on sn-devel-144
2016-04-26 08:09:25 +02:00
Volker Lendecke
d1235c79ec ldb: Fix CID 1348110 Uninitialized scalar variable
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Jan 15 07:12:06 CET 2016 on sn-devel-144
2016-01-15 07:12:06 +01:00
Andrew Bartlett
a44e4e9323 ldb: validate ldb_dn_set_component input parameters even more strictly
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jelmer Vernooij <jelmer@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Jan  6 00:33:21 CET 2016 on sn-devel-144
2016-01-06 00:33:21 +01:00
Andrew Bartlett
30e92d0a32 ldb: Explain why this use of talloc_memdup() is safe
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jelmer Vernooij <jelmer@samba.org>
2016-01-05 21:29:06 +01:00
Andrew Bartlett
084bab5a06 ldb: Be strict about talloc_memdup() and passed in buffers in ldb_dn_set_component()
This ensures we do not over-read the source buffer, but still NUL terminate.

This may be related to debuain bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=808769

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jelmer Vernooij <jelmer@samba.org>
2016-01-05 21:29:06 +01:00
Douglas Bagnall
f36cb71c33 CVE-2015-5330: ldb_dn_explode: copy strings by length, not terminators
That is, memdup(), not strdup(). The terminators might not be there.

But, we have to make sure we put the terminator on, because we tend to
assume the terminator is there in other places.

Use talloc_set_name_const() on the resulting chunk so talloc_report()
remains unchanged.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
Pair-programmed-with: Garming Sam <garming@catalyst.net.nz>
Pair-programmed-with: Stefan Metzmacher <metze@samba.org>
Pair-programmed-with: Ralph Boehme <slow@samba.org>
2015-12-09 17:19:53 +01:00
Douglas Bagnall
0454b95657 CVE-2015-5330: ldb_dn_escape_value: use known string length, not strlen()
ldb_dn_escape_internal() reports the number of bytes it copied, so
lets use that number, rather than using strlen() and hoping a zero got
in the right place.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2015-12-09 17:19:52 +01:00
Douglas Bagnall
7f51ec8c4e CVE-2015-5330: ldb_dn: simplify and fix ldb_dn_escape_internal()
Previously we relied on NUL terminated strings and jumped back and
forth between copying escaped bytes and memcpy()ing un-escaped chunks.
This simple version is easier to reason about and works with
unterminated strings. It may also be faster as it avoids reading the
string twice (first with strcspn, then with memcpy).

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
2015-12-09 17:19:51 +01:00
Andrew Bartlett
1c02f2801e ldb: Fix python bindings to accept a string as a DN
This fixes add_base(), add_child() and is_child_of().

This removes a toally incorrect cast of struct ldb_dn to struct ldb_context.

A helper routine is used instead

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2015-10-26 05:11:21 +01:00
Christian Ambach
9784ed9fb7 lib/ldb fix compiler warnings
about potentially uninitialized variables

Signed-off-by: Christian Ambach <ambi@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2013-12-12 14:21:27 -08:00
Matthieu Patou
057896a090 ldb: use strncmp instead of strcmp when comparing the val part
val part of a DN's component is DATA_BLOB and nothing insure that it
will be finished by a '\0'

Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2013-02-08 15:06:37 +11:00
Andrew Tridgell
4ba8069e3d ldb: added ldb_dn_replace_components()
this allows you to replace the string part of a DN with the string
part from another DN. This is useful when you want to fix a DN that
has the right GUID but the wrong string part, because the target
object has moved.

Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
Pair-Programmed-With: Amitay Isaacs <amitay@gmail.com>
2011-08-04 16:17:24 +10:00
Andrew Tridgell
58e89443e2 ldb: don't shortcut dn comparison for mismatched special DNs
DNs that start with @ can't be compared via string comparison with
normal DNs

Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
2011-07-13 12:51:05 +02:00
Andrew Bartlett
8420a36dc7 ldb: make ldb a top level library for Samba 4.0
Signed-off-by: Andrew Tridgell <tridge@samba.org>
2011-07-05 17:24:47 +10:00