The branch, master has been updated
via f25e8ccf0d1 CVE-2025-10230: s4:wins: restrict names fed to shell
via 90b01ac9029 CVE-2025-10230: s4/tests: check that wins hook
sanitizes names
from 2af8904b3bd smbd: Simplify smb2_parse_file_rename_information()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit f25e8ccf0d17fac19f6059ab91534485c8a3ad5a
Author: Douglas Bagnall <[email protected]>
Date: Wed Sep 3 14:20:24 2025 +1200
CVE-2025-10230: s4:wins: restrict names fed to shell
If the "wins hook" smb.conf parameter is set, the WINS server will
attempt to execute that value in a shell command line when a client
asks to modify a name. The WINS system is a trusting one, and clients
can claim any NETBIOS name they wish.
With the source3 nmbd WINS server (since the 1999 commit now called
3db52feb1f3b2c07ce0b06ad4a7099fa6efe3fc7) the wins hook will not be
run for names that contain shell metacharacters. This restriction has
not been present on the source4 nbt WINS server, which is the WINS
server that will be used in the event that an Active Directory Domain
Controller is also running WINS.
This allowed an unauthenticated client to execute arbitrary commands
on the server.
This commit brings the nmbd check into the nbt WINS server, so that
the wins hook will only be run for names that contain only letters,
digits, hyphens, underscores and periods. This matches the behaviour
described in the smb.conf man page.
The source3 nmbd WINS server has another layer of protection, in that
it uses the smb_run() exec wrapper that tries to escape arguments. We
don't do that here.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15903
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Gary Lockyer <[email protected]>
Autobuild-User(master): Douglas Bagnall <[email protected]>
Autobuild-Date(master): Tue Oct 21 19:43:25 UTC 2025 on atb-devel-224
commit 90b01ac9029169f0a185e74233a71b19a1b4acf0
Author: Douglas Bagnall <[email protected]>
Date: Tue Sep 9 13:36:16 2025 +1200
CVE-2025-10230: s4/tests: check that wins hook sanitizes names
An smb.conf can contain a 'wins hook' parameter, which names a script
to run when a WINS name is changed. The man page says
The second argument is the NetBIOS name. If the name is not a
legal name then the wins hook is not called. Legal names contain
only letters, digits, hyphens, underscores and periods.
but it turns out the legality check is not performed if the WINS
server in question is the source4 nbt one. It is not expected that
people will run this server, but they can. This is bad because the
name is passed unescaped into a shell command line, allowing command
injection.
For this test we don't care whether the WINS server is returning an
error code, just whether it is running the wins hook. The tests show
it often runs the hook it shouldn't, though some characters are
incidentally blocked because the name has to fit in a DN before it
gets to the hook, and DNs have a few syntactic restrictions (e.g.,
blocking '<', '>', and ';').
The source3 WINS server that is used by Samba when not run as a DC is
not affected and not here tested.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15903
Signed-off-by: Douglas Bagnall <[email protected]>
Reviewed-by: Gary Lockyer <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
python/samba/tests/usage.py | 2 +
selftest/target/Samba4.pm | 1 +
source4/nbt_server/wins/wins_hook.c | 9 +++
source4/torture/nbt/wins.c | 136 +++++++++++++++++++++++++++++++++++-
testprogs/blackbox/wins_hook_test | 15 ++++
5 files changed, 160 insertions(+), 3 deletions(-)
create mode 100755 testprogs/blackbox/wins_hook_test
Changeset truncated at 500 lines:
diff --git a/python/samba/tests/usage.py b/python/samba/tests/usage.py
index eb43bba64f4..dae71ecfda8 100644
--- a/python/samba/tests/usage.py
+++ b/python/samba/tests/usage.py
@@ -73,6 +73,7 @@ EXCLUDE_USAGE = {
'lib/ldb/tests/python/api.py',
'source4/selftest/tests.py',
'buildtools/bin/waf',
+ 'testprogs/blackbox/wins_hook_test',
'selftest/tap2subunit',
'script/show_test_time',
'source4/scripting/bin/subunitrun',
@@ -89,6 +90,7 @@ EXCLUDE_HELP = {
'selftest/tap2subunit',
'wintest/test-s3.py',
'wintest/test-s4-howto.py',
+ 'testprogs/blackbox/wins_hook_test',
}
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 1ab178489b2..9b0bce4e26d 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -1637,6 +1637,7 @@ sub provision_ad_dc_ntvfs($$$)
ldap server require strong auth =
allow_sasl_without_tls_channel_bindings
raw NTLMv2 auth = yes
lsa over netlogon = yes
+ wins hook = $ENV{SRCDIR_ABS}/testprogs/blackbox/wins_hook_test
rpc server port = 1027
auth event notification = true
dsdb event notification = true
diff --git a/source4/nbt_server/wins/wins_hook.c
b/source4/nbt_server/wins/wins_hook.c
index 1af471b15bc..442141fecdd 100644
--- a/source4/nbt_server/wins/wins_hook.c
+++ b/source4/nbt_server/wins/wins_hook.c
@@ -43,9 +43,18 @@ void wins_hook(struct winsdb_handle *h, const struct
winsdb_record *rec,
int child;
char *cmd = NULL;
TALLOC_CTX *tmp_mem = NULL;
+ const char *p = NULL;
if (!wins_hook_script || !wins_hook_script[0]) return;
+ for (p = rec->name->name; *p; p++) {
+ if (!(isalnum((int)*p) || strchr_m("._-", *p))) {
+ DBG_ERR("not calling wins hook for invalid name %s\n",
+ rec->name->name);
+ return;
+ }
+ }
+
tmp_mem = talloc_new(h);
if (!tmp_mem) goto failed;
diff --git a/source4/torture/nbt/wins.c b/source4/torture/nbt/wins.c
index 8c847b5ac50..7d7321752d6 100644
--- a/source4/torture/nbt/wins.c
+++ b/source4/torture/nbt/wins.c
@@ -31,6 +31,10 @@
#include "torture/nbt/proto.h"
#include "param/param.h"
+/* rcode used when you don't want to check the rcode */
+#define WINS_TEST_RCODE_WE_DONT_CARE 255
+
+
#define CHECK_VALUE(tctx, v, correct) \
torture_assert_int_equal(tctx, v, correct, "Incorrect value")
@@ -137,7 +141,9 @@ static bool nbt_test_wins_name(struct torture_context
*tctx, const char *address
address));
CHECK_STRING(tctx, io.out.wins_server, address);
- CHECK_VALUE(tctx, io.out.rcode, 0);
+ if (register_rcode != WINS_TEST_RCODE_WE_DONT_CARE) {
+ CHECK_VALUE(tctx, io.out.rcode, 0);
+ }
torture_comment(tctx, "register the name correct address\n");
name_register.in.name = *name;
@@ -185,7 +191,9 @@ static bool nbt_test_wins_name(struct torture_context
*tctx, const char *address
talloc_asprintf(tctx, "Bad response from %s for name
register\n",
address));
- CHECK_VALUE(tctx, name_register.out.rcode, 0);
+ if (register_rcode != WINS_TEST_RCODE_WE_DONT_CARE) {
+ CHECK_VALUE(tctx, name_register.out.rcode, 0);
+ }
CHECK_STRING(tctx, name_register.out.reply_addr, myaddress);
}
@@ -203,7 +211,9 @@ static bool nbt_test_wins_name(struct torture_context
*tctx, const char *address
torture_assert_ntstatus_ok(tctx, status, talloc_asprintf(tctx, "Bad
response from %s for name register", address));
CHECK_STRING(tctx, io.out.wins_server, address);
- CHECK_VALUE(tctx, io.out.rcode, register_rcode);
+ if (register_rcode != WINS_TEST_RCODE_WE_DONT_CARE) {
+ CHECK_VALUE(tctx, io.out.rcode, register_rcode);
+ }
if (register_rcode != NBT_RCODE_OK) {
return true;
@@ -532,6 +542,124 @@ static bool nbt_test_wins(struct torture_context *tctx)
return ret;
}
+/*
+ * Test that the WINS server does not call 'wins hook' when the name
+ * contains dodgy characters.
+ */
+static bool nbt_test_wins_bad_names(struct torture_context *tctx)
+{
+ const char *address = NULL;
+ const char *wins_hook_file = NULL;
+ bool ret = true;
+ int err;
+ bool ok;
+ struct nbt_name name = {};
+ size_t i, j;
+ FILE *fh = NULL;
+
+ struct {
+ const char *name;
+ bool should_succeed;
+ } test_cases[] = {
+ {"NORMAL", true},
+ {"|look|", false},
+ {"look&true", false},
+ {"look\\;false", false},
+ {"&ls>foo", false}, /* already fails due to DN syntax */
+ {"has spaces", false},
+ {"hyphen-dot.0", true},
+ };
+
+ wins_hook_file = talloc_asprintf(tctx, "%s/wins_hook_writes_here",
+ getenv("SELFTEST_TMPDIR"));
+
+ if (!torture_nbt_get_name(tctx, &name, &address)) {
+ return false;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
+ err = unlink(wins_hook_file);
+ if (err != 0 && errno != ENOENT) {
+ /* we expect ENOENT, but nothing else */
+ torture_comment(tctx,
+ "unlink %zu of '%s' failed\n",
+ i, wins_hook_file);
+ }
+
+ name.name = test_cases[i].name;
+ name.type = NBT_NAME_CLIENT;
+ ok = nbt_test_wins_name(tctx, address,
+ &name,
+ NBT_NODE_H,
+ true,
+ WINS_TEST_RCODE_WE_DONT_CARE
+ );
+ if (ok == false) {
+ /*
+ * This happens when the name interferes with
+ * the DN syntax when it is put in winsdb.
+ *
+ * The wins hook will not be reached.
+ */
+ torture_comment(tctx, "tests for '%s' failed\n",
+ name.name);
+ }
+
+ /*
+ * poll for the file being created by the wins hook.
+ */
+ for (j = 0; j < 10; j++) {
+ usleep(200000);
+ fh = fopen(wins_hook_file, "r");
+ if (fh != NULL) {
+ break;
+ }
+ }
+
+ if (fh == NULL) {
+ if (errno == ENOENT) {
+ if (test_cases[i].should_succeed) {
+ torture_comment(
+ tctx,
+ "wins hook for '%s' failed\n",
+ test_cases[i].name);
+ ret = false;
+ }
+ } else {
+ torture_comment(
+ tctx,
+ "wins hook for '%s' unexpectedly failed
with %d\n",
+ test_cases[i].name,
+ errno);
+ ret = false;
+ }
+ } else {
+ char readback[17] = {0};
+ size_t n = fread(readback, 1, 16, fh);
+ torture_comment(tctx,
+ "wins hook wrote '%s' read '%.*s'\n",
+ test_cases[i].name,
+ (int)n, readback);
+
+ if (! test_cases[i].should_succeed) {
+ torture_comment(tctx,
+ "wins hook for '%s' should
fail\n",
+ test_cases[i].name);
+ ret = false;
+ }
+ fclose(fh);
+ }
+ }
+ err = unlink(wins_hook_file);
+ if (err != 0 && errno != ENOENT) {
+ torture_comment(tctx, "final unlink of '%s' failed\n",
+ wins_hook_file);
+ }
+ torture_assert(tctx, ret, "wins hook failure\n");
+ return ret;
+}
+
+
/*
test WINS operations
*/
@@ -540,6 +668,8 @@ struct torture_suite *torture_nbt_wins(TALLOC_CTX *mem_ctx)
struct torture_suite *suite = torture_suite_create(mem_ctx, "wins");
torture_suite_add_simple_test(suite, "wins", nbt_test_wins);
+ torture_suite_add_simple_test(suite, "wins_bad_names",
+ nbt_test_wins_bad_names);
return suite;
}
diff --git a/testprogs/blackbox/wins_hook_test
b/testprogs/blackbox/wins_hook_test
new file mode 100755
index 00000000000..f15379c28ca
--- /dev/null
+++ b/testprogs/blackbox/wins_hook_test
@@ -0,0 +1,15 @@
+#!/usr/bin/python3
+
+import os
+import sys
+
+filename = f"{os.environ['SELFTEST_TMPDIR']}/wins_hook_writes_here"
+
+f = open(filename, 'wb')
+
+# Some names may truncate argv (e.g. '&'), for which we leave the file
+# empty.
+if len(sys.argv) > 2:
+ f.write(os.fsencode(sys.argv[2]))
+
+f.close()
--
Samba Shared Repository
