"Error: UNINIT (CWE-457):
samba-4.20.0rc2/examples/libsmbclient/testacl.c:35: var_decl: Declaring variable ""value"" without initializer.
samba-4.20.0rc2/examples/libsmbclient/testacl.c:254: uninit_use_in_call: Using uninitialized value ""*value"" as argument to ""%s"" when calling ""printf"". [Note: The source code implementation of the function has been overridden by a builtin model.]
252| }
253|
254|-> printf(""Attributes for [%s] are:\n%s\n"", path, value);
255|
256| if (stat_and_retry)"
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Volker Lendecke <vl@samba.org>
"Error: INTEGER_OVERFLOW (CWE-190):
samba-4.20.0rc2/source3/registry/regfio.c:175: tainted_data_argument: The check ""bytes_read < block_size"" contains the tainted expression ""bytes_read"" which causes ""block_size"" to be considered tainted.
samba-4.20.0rc2/source3/registry/regfio.c:176: overflow: The expression ""block_size - bytes_read"" is deemed overflowed because at least one of its arguments has overflowed.
samba-4.20.0rc2/source3/registry/regfio.c:176: overflow_sink: ""block_size - bytes_read"", which might have underflowed, is passed to ""read(file->fd, buffer + bytes_read, block_size - bytes_read)"". [Note: The source code implementation of the function has been overridden by a builtin model.]
174|
175| while ( bytes_read < block_size ) {
176|-> if ( (returned = read( file->fd, buffer+bytes_read, block_size-bytes_read )) == -1 ) {
177| DEBUG(0,(""read_block: read() failed (%s)\n"", strerror(errno) ));
178| return False;"
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Volker Lendecke <vl@samba.org>
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Mon Jul 8 06:28:47 UTC 2024 on atb-devel-224
Jones Syue [Fri, 5 Jul 2024 09:36:46 +0000 (17:36 +0800)]
s3:ntlm_auth: make logs more consistent with length check
Run ntlm_auth with options --lm-response/--nt-response/--challenge, and pass
wrong length to these options, got error prompted logs about 'only got xxx
bytes', which are not consistent with length check. This patch revise logs
for length check to make it more consistent.
For example --lm-response requires exact 24 hex, let us input three kinds
of length 23 24 25, prompted logs said 'only got 25 bytes' seems confusing.
script:
for length in 23 24 25; \
do \
ntlm_auth --username=${un} --password=${pw} \
--lm-response="`openssl rand -hex ${length}`"; \
done;
Signed-off-by: Jones Syue <jonessyue@qnap.com> Reviewed-by: David Mulder <dmulder@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Sat Jul 6 00:52:02 UTC 2024 on atb-devel-224
Signed-off-by: Ralph Boehme <slow@samba.org> Signed-off-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Jul 5 10:02:26 UTC 2024 on atb-devel-224
libcli: New routine symlink_target_path for [MS-SMB2] 2.2.2.2.1.1
Right now the only user is the user-space symlink following in
smbd. We will use it in libsmb as well to correctly handle
STOPPED_ON_SYMLINK. When trying to upstream that code I found the
previous_slash function incredibly hard to understand.
This new routine makes copy of "const char *_name_in", so that we can
replace previous_slash with a simple strrchr_m. If that's too
slow (which I doubt, this is "only" chasing symlinks) we can always do
something smarter again.
Signed-off-by: Volker Lendecke <vl@samba.org> Reviewed-by: Andreas Schneider <asn@samba.org>
gitlab-ci: Also add the git directory for pipeline in the main mirror
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Jo Sutton <josutton@catalyst.net.nz>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Thu Jul 4 08:08:49 UTC 2024 on atb-devel-224
Wireshark's NDR dissector dissects both signed and unsigned types
of the same size and alignment with the same functions, e.g. see
the handling of "udlong" and "dlong." It is passing the FT_UINT64
vs FT_INT64 field type enum value that determines at the last
moment whether a value is cast to signed. dissect_ndr_uint64()
already has the proper behavior for 8-byte aligned signed 64 bit
integers, and a dissect_ndr_int64() function will not need to be
introduced.
Signed-off-by: John Thacker <johnthacker@gmail.com> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Jo Sutton <josutton@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Wed Jul 3 14:19:04 UTC 2024 on atb-devel-224
smbd: correctly restore ENOENT if fstatfs() modifies it
Review with: git show -U5
Signed-off-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Wed Jul 3 11:41:12 UTC 2024 on atb-devel-224
Fix the type of arrays of pointers to hf_ values for bitfield routines.
The static arrays are supposed to be arrays of const pointers to int,
not arrays of non-const pointers to const int.
Fixing that means some bugs (scribbling on what's *supposed* to be a
const array) will be caught (see packet-ieee80211-radiotap.c for
examples, the first of which inspired this change and the second of
which was discovered while testing compiles with this change), and
removes the need for some annoying casts.
Also make some of those arrays static while we're at it.
Update documentation and dissector-generator tools.
Change-Id: I789da5fc60aadc15797cefecfd9a9fbe9a130ccc
Reviewed-on: https://code.wireshark.org/review/37517
Petri-Dish: Guy Harris <gharris@sonic.net> Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman <a.broman58@gmail.com> Signed-off-by: John Thacker <johnthacker@gmail.com> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
Autobuild-Date(master): Wed Jul 3 02:35:43 UTC 2024 on atb-devel-224
Rename tvb_new_subset() to tvb_new_subset_length_caplen().
This emphasizes that there is no such thing as *the* routine to
construct a subset tvbuff; you need to choose one of
tvb_new_subset_remaining() (if you want a new tvbuff that contains
everything past a certain point in an existing tvbuff),
tvb_new_subset_length() (if you want a subset that contains everything
past a certain point, for some number of bytes, in an existing tvbuff),
and tvb_new_subset_length_caplen() (for all other cases).
Many of the calls to tvb_new_subset_length_caplen() should really be
calling one of the other routines; that's the next step. (This also
makes it easier to find the calls that need fixing.)
Change-Id: Ieb3d676d8cda535451c119487d7cd3b559221f2b
Reviewed-on: https://code.wireshark.org/review/19597 Reviewed-by: Guy Harris <guy@alum.mit.edu> Signed-off-by: John Thacker <johnthacker@gmail.com> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Get rid of Boolean "flags" that don't have any bit set.
And tweak the Pidl generator for Wireshark not to generate "flags" like
that.
(The generator also does field name and true/false strings' case
differently, so I didn't use it to regenerate the dissectors; that needs
to be looked at.)
Change-Id: Ie1657a782ebdb107e58792cedd29bbaa79b17bd4
Reviewed-on: https://code.wireshark.org/review/18695 Reviewed-by: Guy Harris <guy@alum.mit.edu> Signed-off-by: John Thacker <johnthacker@gmail.com> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Change-Id: Ie89f8fd4ec744d427d41866206d5a6784c5b224f
Reviewed-on: https://code.wireshark.org/review/16004
Petri-Dish: Jaap Keuter <jaap.keuter@xs4all.nl> Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org> Reviewed-by: Michael Mann <mmann78@netscape.net> Signed-off-by: John Thacker <johnthacker@gmail.com> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Change-Id: I8891ec90244ffd9609d8443df631a7c8e6453b7e
Reviewed-on: https://code.wireshark.org/review/15942
Petri-Dish: Michael Mann <mmann78@netscape.net> Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org> Reviewed-by: Michael Mann <mmann78@netscape.net> Signed-off-by: John Thacker <johnthacker@gmail.com> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org>
Douglas Bagnall [Sat, 29 Jun 2024 01:43:03 +0000 (13:43 +1200)]
cmdline: test_cmdline tests more burning
We have more secret arguments, like --client-password, --adminpass,
so we are going to use an allowlist for options containing 'pass', but
we don't want to burn the likes of --group=passionfruit.
Douglas Bagnall [Thu, 27 Jun 2024 04:03:30 +0000 (16:03 +1200)]
cmdline:burn: always return true if burnt
Before we have been trying to cram three cases into a boolean return
value:
* cmdline had secrets, we burnt them -> true
* cmdline had no secrets, all good -> false
* cmdline has NULL string, WTF! emergency! -> false
This return value is only used by Python which wants to know whether to
go to the trouble of replacing the command line. If samba_cmdline_burn()
returns false, no action is taken.
If samba_cmdline_burn() burns a password and then hits a NULL, it would
be better not to do nothing. It would be better to crash. And that is
what Python will end up doing, by some talloc returning NULL triggering
a MemoryError.
What about the case like {"--foo", NULL, "-Ua%b"} where the secret comes
after the NULL? That will still be ignored by Python, as it is by all C
tools, but we are hoping that can't happen anyway.
Douglas Bagnall [Thu, 27 Jun 2024 03:20:27 +0000 (15:20 +1200)]
cmdline:burn: do not retain false memories
If argv contains a secret option without an '=' (or in the case of
"-U", the username is separated by space), we will get to the
`if (strlen(p) == ulen) { continue; }` without resetting the found
and is_user variables. This *sometimes* has the right effect, because
the next string in argv ought to contain the secret.
But in a case like {"--password", "1234567890"}, where the secret
string is the same length as the option, we *again* take that branch
and the password is not redacted, though the argument after it will be
unless it is also of the same length.
If we always set the flags at the start we avoid this. This makes
things worse in the short term for secrets that are not the same
length as their options, but we'll get to that in another commit soon.
Douglas Bagnall [Thu, 20 Jun 2024 23:29:36 +0000 (11:29 +1200)]
docs-xml:manpages: allow for longer version strings
The default value (30) truncates "Samba 4.21.0pre1-DEVELOPERBUILD" to
"Samba 4.21.0pre1-DEVELOPE" in the bottom left corner of the man page.
("Samba 4.21.0pre1-DEVELOPE" is only 25 bytes long, not 30, but let's
not worry about that).
On narrow terminals (< ~75 columns) this makes it more likely that
the version string will run into the date string.
Signed-off-by: Björn Baumbach <bb@sernet.de> Reviewed-by: Volker Lendecke <vl@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Jo Sutton <josutton@catalyst.net.nz> Reviewed-by: Martin Schwenke <martin@meltin.net>
Autobuild-User(master): Martin Schwenke <martins@samba.org>
Autobuild-Date(master): Tue Jul 2 23:52:37 UTC 2024 on atb-devel-224
Jo Sutton [Mon, 1 Jul 2024 03:55:13 +0000 (15:55 +1200)]
s3:smbd: Avoid compiler warning for unused label
If either of HAVE_FSTATFS and HAVE_LINUX_MAGIC_H are not defined, gcc
produces the following error:
../../source3/smbd/open.c: In function ‘reopen_from_fsp’:
../../source3/smbd/open.c:1222:1: error: label ‘namebased_open’ defined but not used [-Werror=unused-label]
1222 | namebased_open:
| ^~~~~~~~~~~~~~
Signed-off-by: Jo Sutton <josutton@catalyst.net.nz> Reviewed-by: Martin Schwenke <martin@meltin.net>
Autobuild-User(master): Martin Schwenke <martins@samba.org>
Autobuild-Date(master): Tue Jul 2 04:44:56 UTC 2024 on atb-devel-224
Xavi Hernandez [Thu, 27 Jun 2024 13:41:19 +0000 (15:41 +0200)]
Fix starvation of pending writes in CTDB queues
CTDB uses a queue to receive requests and send answers. It works
asynchronously using the tevent framework. However there was an issue
that gave priority to the receiving side so, when a request was
processed and the answer posted to the queue, if another incoming
request arrived, it was served before sending the previous answer.
This scenario could repeat for long periods of time if the frequency of
incoming requests was high enough.
Eventually, a small time gap between incoming request gave a chance to
process the pending output queue, sending many answers in a burst.
This patch makes sure that both queues (input and output) are processed
if the event contains the appropriate flag.
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com> Reviewed-by: Volker Lendecke <vl@samba.org> Reviewed-by: Martin Schwenke <martin@meltin.net>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Mon Jul 1 09:17:43 UTC 2024 on atb-devel-224
"Error: UNINIT (CWE-457):
samba-4.20.0rc2/source4/torture/gentest.c:2690: var_decl: Declaring variable ""parm"" without initializer.
samba-4.20.0rc2/source4/torture/gentest.c:2711: uninit_use: Using uninitialized value ""parm[0]"". Field ""parm[0].out"" is uninitialized.
2709| }
2710|
2711|-> GEN_COPY_PARM;
2712| GEN_SET_FNUM_SMB2(in.file.handle);
2713| GEN_CALL_SMB2(smb2_lock(tree, &parm[i]));"
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Martin Schwenke <mschwenke@ddn.com>
Autobuild-User(master): Martin Schwenke <martins@samba.org>
Autobuild-Date(master): Mon Jul 1 00:23:08 UTC 2024 on atb-devel-224
"Error: INTEGER_OVERFLOW (CWE-190):
samba-4.20.0rc2/source3/winbindd/winbindd_cache.c:849: cast_overflow: Truncation due to cast operation on ""len"" from 32 to 8 bits.
samba-4.20.0rc2/source3/winbindd/winbindd_cache.c:851: overflow_sink: ""len"", which might have overflowed, is passed to ""memcpy(centry->data + centry->ofs, s, len)"". [Note: The source code implementation of the function has been overridden by a builtin model.]
849| centry_put_uint8(centry, len);
850| centry_expand(centry, len);
851|-> memcpy(centry->data + centry->ofs, s, len);
852| centry->ofs += len;
853| }"
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Martin Schwenke <mschwenke@ddn.com>
s3:libsmb: Check if we have a valid file descriptor
"Error: REVERSE_NEGATIVE (CWE-191):
samba-4.20.0rc2/source3/libsmb/pylibsmb.c:215: negative_sink_in_call: Passing ""t->shutdown_pipe[1]"" to a parameter that cannot be negative.
samba-4.20.0rc2/source3/libsmb/pylibsmb.c:230: check_after_sink: You might be using variable ""t->shutdown_pipe[1]"" before verifying that it is >= 0.
228| t->shutdown_pipe[0] = -1;
229| }
230|-> if (t->shutdown_pipe[1] != -1) {
231| close(t->shutdown_pipe[1]);
232| t->shutdown_pipe[1] = -1;"
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Martin Schwenke <mschwenke@ddn.com>
examples: Make sure the array is probably initialized
"Error: UNINIT (CWE-457):
samba-4.20.0rc2/examples/libsmbclient/testacl2.c:27: var_decl: Declaring variable ""value"" without initializer.
samba-4.20.0rc2/examples/libsmbclient/testacl2.c:48: uninit_use_in_call: Using uninitialized value ""*value"" as argument to ""%s"" when calling ""printf"". [Note: The source code implementation of the function has been overridden by a builtin model.]
46| }
47|
48|-> printf(""Attributes for [%s] are:\n%s\n"", argv[1], value);
49|
50| flags = 0;"
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Martin Schwenke <mschwenke@ddn.com>
Volker Lendecke [Mon, 24 Jun 2024 14:50:57 +0000 (16:50 +0200)]
smbd: Fix cached dos attributes
The callers of fset_dos_mode must set the cached attributes
themselves, which I did not see. I tried, but I did not find a clean
way to fix this behind SMB_VFS_FSET_DOS_ATTRIBUTES, with a smb_fname
and smb_fname->fsp->fsp_name we might have two copies of the cached
dos attributes around and if we only update fsp->fsp_name, we might
miss the outer one.
Not doing a test, this is really fresh code, and in the future we must
reorganize setting and caching dos attributes anyway.
Signed-off-by: Volker Lendecke <vl@samba.org> Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Jun 28 14:32:27 UTC 2024 on atb-devel-224