1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-23 17:34:34 +03:00
Commit Graph

439 Commits

Author SHA1 Message Date
Volker Lendecke
7528b7886b librpc: Fix a typo
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Samuel Cabrero <scabrero@samba.org>
2021-01-28 16:58:35 +00:00
Volker Lendecke
2b9ba992b6 lib: Simplify parse_guid_string() and ndr_syntax_id_from_string()
Return "bool" instead of NTSTATUS, use hex_byte() instead of
read_hex_bytes(). And parse directly into a struct GUID instead of the
components. 99 lines less code.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Samuel Cabrero <scabrero@samba.org>
2021-01-28 16:58:35 +00:00
Volker Lendecke
a35c8a0ec2 librpc: Add ndr_syntax_id_buf_string()
Same pattern as GUID_buf_string()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Samuel Cabrero <scabrero@samba.org>
2021-01-28 16:58:35 +00:00
Volker Lendecke
9374313f03 libndr: Simplify ndr_print_GUID()
Fix a small memleak of the tmp GUID_string

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Samuel Cabrero <scabrero@samba.org>
2021-01-28 16:58:35 +00:00
Volker Lendecke
29e3c0cdb7 librpc: Use GUID_to_ndr_buf() in GUID_to_ndr_blob()
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-10-02 21:30:33 +00:00
Volker Lendecke
bbb017637e librpc: Add GUID_to_ndr_buf()
Avoids talloc

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-10-02 21:30:33 +00:00
Volker Lendecke
5a143c0931 lib: Avoid a use of includes.h
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-10-02 21:30:33 +00:00
Douglas Bagnall
bf16cd72b2 ndr: fix ndr_pull_string_array() off by one alloc
The correct line should have been

       talloc_realloc(ndr->current_mem_ctx, a, const char *, count + 2);

because if the loop does not increment count on exit (it exits via
break), so count is left pointing at the thing that just got put in.
i.e., if there was one item it is at a[0], count is 0, but we also
need the trailing NULL byte at a[1] and the length is 2. Thus + 2, not
+ 1.

This will not affect ordinary (that is, non-malicious) traffic,
because talloc_realloc will not actually realloc unless it is saving a
kilobyte. Since the allocation grows slowly with the exponent ~1.25,
the actual reallocs will start happening at some point between 512 and
1024 items.

In the example we have, there were 666 pointers, and space for 824 was
allocated.

Rather than doing the +2 realloc, it is simpler to leave it off
altogether; in the common case (<512 items) it is a no-op anyway, and
in the best possible case it reduces the temporary array by 20%.

Credit to OSS-Fuzz.

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=24646

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-08-07 03:23:44 +00:00
Douglas Bagnall
9148f38c20 ndr: avoid excessive reallocing in pull_string_array
Before, talloc_realloc() was being called n times for an array of
length n. This could be very expensive on long string arrays since it
is reasonable to assume each realloc moves O(n) bytes.

This addresses at least one OSS-Fuzz bug, making a timing out test case
100 times faster. Credit to OSS-Fuzz.

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19706

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Noel Power <npower@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-08-03 02:51:35 +00:00
Douglas Bagnall
9bf331b46a ndr: maintain proper talloc tree in pull_string_array
We don't want to leave other parts of the ndr struct hanging off this
string array just because LIBNDR_FLAG_REMAINING is used.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Noel Power <npower@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-08-03 02:51:35 +00:00
Douglas Bagnall
cc3a67760c CVE-2020-10745: ndr/dns-utils: prepare for NBT compatibility
NBT has a funny thing where it sometimes needs to send a trailing dot as
part of the last component, because the string representation is a user
name. In DNS, "example.com", and "example.com." are the same, both
having three components ("example", "com", ""); in NBT, we want to treat
them differently, with the second form having the three components
("example", "com.", "").

This retains the logic of e6e2ec0001.

Also DNS compression cannot be turned off for NBT.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-07-02 09:01:41 +00:00
Douglas Bagnall
c3fa8ada43 CVE-2020-10745: dns_util/push: forbid names longer than 255 bytes
As per RFC 1035.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-07-02 09:01:41 +00:00
Douglas Bagnall
51a4571849 CVE-2020-10745: ndr_dns: do not allow consecutive dots
The empty subdomain component is reserved for the root domain, which we
should only (and always) see at the end of the list. That is, we expect
"example.com.", but never "example..com".

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-07-02 09:01:41 +00:00
Douglas Bagnall
bb63737905 CVE-2020-10745: ndr/dns_utils: correct a comment
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-07-02 09:01:41 +00:00
Douglas Bagnall
601e8a3f69 CVE-2020-10745: ndr_dns: move ndr_push_dns_string core into sharable function
This is because ndr_nbt.c does almost exactly the same thing with
almost exactly the same code, and they both do it wrong. Soon they
will both be using the better version that this will become. Though in
this patch we just move the code, not fix it.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-07-02 09:01:41 +00:00
Gary Lockyer
ba518a1deb librpc ndr: add recursion check macros
Add macros to check the recursion depth.

Credit to OSS-Fuzz

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19280
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14254

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-27 01:02:32 +00:00
Andreas Schneider
7ab1156b1e librpc:ndr: Remove byteorder.h from libndr.h
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-21 02:09:33 +00:00
Andreas Schneider
5268c83a48 librpc:ndr: Use bytearray.h in ndr_basic.c
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-21 02:09:33 +00:00
Gary Lockyer
91d4e79c27 librpc ndr: Change loop index to size_t
Change the loop index in ndr_check_padding to size_t.

Credit to OSS-Fuzz

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-07 08:53:40 +00:00
Gary Lockyer
14182350f8 librpc ndr: ndr_pull_advance check for unsigned overflow.
Handle uint32 overflow in ndr_pull_advance

Credit to OSS-Fuzz

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-07 08:53:40 +00:00
Gary Lockyer
6d05fb3ea7 librpc ndr: NDR_PULL_ALIGN check for unsigned overflow
Handle uint32 overflow in NDR_PULL_ALIGN

Credit to OSS-Fuzz

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-07 08:53:40 +00:00
Gary Lockyer
ae6927e4f0 librpc ndr: Heap-buffer-overflow in lzxpress_decompress
Reproducer for oss-fuzz Issue 20083

Project: samba
Fuzzing Engine: libFuzzer
Fuzz Target: fuzz_ndr_drsuapi_TYPE_OUT
Job Type: libfuzzer_asan_samba
Platform Id: linux

Crash Type: Heap-buffer-overflow READ 1
Crash Address: 0x6040000002fd
Crash State:
  lzxpress_decompress
    ndr_pull_compression_xpress_chunk
      ndr_pull_compression_start

Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Credit to OSS-Fuzz

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14236

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2020-02-07 08:53:40 +00:00
Volker Lendecke
a0b967ec15 librpc: Use PRIu32 printf specifiers
Both values printed are uint32_t's

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-01-23 19:11:34 +00:00
Volker Lendecke
4864a4bb96 librpc: Use the MAX() macro where appropriate
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-01-23 19:11:34 +00:00
Volker Lendecke
7b5edaebce librpc: Fix a gcc warning
gcc does not like const before static

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-01-23 19:11:34 +00:00
Gary Lockyer
86a31defbf ndr basic: Check ndr_token_store return code
Fix for

*** CID 1457529:  Error handling issues  (CHECKED_RETURN)
/librpc/ndr/ndr_basic.c: 786 in ndr_push_full_ptr()
...
ID 1457529:  Error handling issues  (CHECKED_RETURN)
    Calling "ndr_token_store" without checking return value (as is done
    elsewhere 14 out of 17 times).

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: David Disseldorp <ddiss@samba.org>

Autobuild-User(master): Gary Lockyer <gary@samba.org>
Autobuild-Date(master): Thu Jan  9 21:49:00 UTC 2020 on sn-devel-184
2020-01-09 21:49:00 +00:00
Günther Deschner
c433dac598 librpc: pidlify spoolss_EnumPerMachineConnections
Guenther

Signed-off-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-01-08 23:51:31 +00:00
Günther Deschner
c9bebbcc46 librpc/ndr: provide infrastructure for NDR_SPOOLSS_{PUSH|PULL}_ENUM macros
The new macros allow the same macro convenience for enum functions without
level discriminator.

Guenther

Signed-off-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-01-08 23:51:31 +00:00
Günther Deschner
1038613878 librpc: better naming for NDR_SPOOLSS_{PUSH|PULL}_ENUM variants
Guenther

Signed-off-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2020-01-08 23:51:30 +00:00
Andrew Bartlett
a85d257c1e librpc: Do not access name[-1] trying to push "" into a dnsp_name
This simply matches the behaviour from before e7b1acaddf
when the logic for a trailing . was added.  This matches what is added in
the dnsRecord attribute for a name of "." over the dnsserver RPC
management interface and is based on what Windows does for that name
in (eg) an MX record.

No a security bug because we use talloc and so name will be just the
end of the talloc header.

Credit to OSS-Fuzz

Found using the fuzz_ndr_X fuzzer

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Dec 20 11:33:52 UTC 2019 on sn-devel-184
2019-12-20 11:33:52 +00:00
Andreas Schneider
f11e207e01 librpc: Fix string length checking in ndr_pull_charset_to_null()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14219

Pair-Programmed-With: Guenther Deschner <gd@samba.org>
Signed-off-by: Guenther Deschner <gd@samba.org>
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2019-12-20 07:35:41 +00:00
Andrew Bartlett
0e651b4508 librpc: Fix manually written printer for drsuapi_DsAttributeValue
Credit to OSS-Fuzz

Found using the ndr_fuzz_X target.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-18 06:39:26 +00:00
Andrew Bartlett
82aff583b7 libndr: Return enum ndr_err_code from ndr_{pull,push}_steal_switch_value()
This breaks the ABI so we merge this into the unreleased libndr-1.0.0.

The advantage of the new functions is there (except for print, which
is unchanged) is an error raised when the token is not found, so
we can be confident in the changes to the token behaviour.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Thu Dec 12 03:56:23 UTC 2019 on sn-devel-184
2019-12-12 03:56:23 +00:00
Andrew Bartlett
d30c377d8c librpc/ndr: Remove ndr_{push,pull}_get_switch_value()
By removing this we know we do not need to worry about this list
growing without bounds.  We merge this into the recently created but
not yet released ABI 1.0.0

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:41 +00:00
Andrew Bartlett
a44717394d librpc: Remove last callers of ndr_push_get_switch_value()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
b880bda589 librpc: Remove last callers of ndr_pull_get_switch_value()
By removing this we know we do not need to worry about this list
growing without bounds.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
92a7c5a726 librpc: Do not follow a NULL pointer when calculating the size of a union
Found by Douglas Bagnall using Hongfuzz and the new fuzz_ndr_X
fuzzer.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
362d70ff2f librpc: Do not follow a NULL pointer when calculating the size of a structure
Found by Douglas Bagnall using Hongfuzz and the new fuzz_ndr_X
fuzzer.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
e08461b284 pidl: Add and use ndr_print_steal_switch_value(), removing ndr_print_get_switch_value()
This avoids really long token lists for switch values
that will not be needed past this point.

The function name is changed to clarify what exactly is being
done here, and the old function is removed to ensure it is
not being used anywhere else.

Merge the removal of ndr_print_get_switch_value into
just-tagged librpc/ABI/ndr-1.0.0.sigs as this
has not been put into any release yet.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
e1a15710a4 librpc/ndr: Add ndr_push_steal_switch_value()
This will allow generated code to instead push and pop union values onto the
switch_list stack, which is more memory efficient than creating a single large
list to be scannned and eventually discarded.

Merge into unreleased ABI 1.0.0

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
f7bcf227f7 librpc: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
a22a22e4a8 negoex: Set the switch_value before NDR_BUFFERS to prepare for new libndr behaviour
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:40 +00:00
Andrew Bartlett
7a0ed44b0e ndr: Restrict size of ndr_token lists to avoid memory abuse by malicious clients
This is designed to stop a very large number of tokens from being stored for
arrays of structures containing relative pointers in particular.

This was one part of the minimum patch for CVE-2019-14908 before
being downgraded as not a security-release worthy issue.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:39 +00:00
Andrew Bartlett
4501663f6e libndr: Do not overwrite token list with NULL on allocation failure
This was one part of the minimum patch for CVE-2019-14908 before
being downgraded as not a security-release worthy issue.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2019-12-12 02:30:39 +00:00
Andrew Bartlett
b6913830bf librpc/ndr: Do not follow a client-supplied NULL pointer in _print_drsuapi_DsAttributeValue_attid()
This is not a security issue as it only happens when printing the structure
during debugging, not normal production.

Found by Michael Hanselmann using an NDR fuzzer and Hongfuzz.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2019-12-10 02:53:34 +00:00
Andrew Bartlett
d15a3797c7 librpc: Avoid spinning on string_array elements with a short input
Without this protection we will spin during decode of a string_array or nstring_array
that is terminated by only a single NUL byte, not two as required by UTF-16.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2019-12-04 05:10:31 +00:00
Volker Lendecke
4cd6c87a3d libndr: Fix a talloc context leak
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2019-12-02 21:23:35 +00:00
Andrew Bartlett
674d2cfd04 librpc/ndr: Remove unused ndr_cab_generate_checksum()
This function is not just unused, it is a problem because there is no
enforced connection between r->ab.length and r->cbData.

The last caller was removed in the previous commit.

Found by Douglas Bagnall using Hongfuzz and the new fuzz_ndr_X
fuzzer.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2019-11-29 00:44:40 +00:00
Douglas Bagnall
536a84935c ndr_orpc: properly allocate empty DUALSTRINGARRAY
When there is no data we still need to allocate for the terminating NULL.

Found by Michael Hanselmann using Honggfuzz and an fuzzer for Samba's
NDR layer.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2019-11-20 04:41:28 +00:00
Andrew Bartlett
42ac80fb46 ndr: Include the caller location in ndr_{pull,push}_error() messages
This should aid in debugging NDR parse failures.

Use the ABI bump to again exclude ndr_table_misc incorrectly
added in ABI 0.2.1.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Nov 19 14:47:46 UTC 2019 on sn-devel-184
2019-11-19 14:47:46 +00:00