The branch, master has been updated via 2868b803649 lib/replace/timegm: use utf-8 via 039f876c4e9 s4/auth/gensec/gensec_krb5_heimdal: use utf-8 via 6ced906e2be test/blackbox/test_samba-tool_ntacl: use utf-8 via 4c85693f553 s3/modules/vfs_acl_common.h: use utf-8 via c3194d0d65d test/bad_chars: ensure our tests could fail via fccb105e079 pytests: check that we don't have bad format characters from 1c8ea2448ea s3: smbd: In SMB1 call_trans2findnext() add and use a helper variable to ensure we don't call mangle_is_mangled() with a posix name.
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 2868b8036498e7fa0c7ae3615f5d97b42b360da2 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Nov 17 09:47:18 2021 +1300 lib/replace/timegm: use utf-8 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Ralph Boehme <s...@samba.org> Autobuild-User(master): Ralph Böhme <s...@samba.org> Autobuild-Date(master): Wed Nov 17 05:27:39 UTC 2021 on sn-devel-184 commit 039f876c4e9f635b207f3b16c99662297a93dd5e Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Nov 17 09:48:37 2021 +1300 s4/auth/gensec/gensec_krb5_heimdal: use utf-8 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Ralph Boehme <s...@samba.org> commit 6ced906e2be66fb324aa012a06c8d3b10bbf78b2 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Nov 17 09:49:05 2021 +1300 test/blackbox/test_samba-tool_ntacl: use utf-8 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Ralph Boehme <s...@samba.org> commit 4c85693f55341344117f0b6d2bb7498099828dab Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Nov 17 09:47:52 2021 +1300 s3/modules/vfs_acl_common.h: use utf-8 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Ralph Boehme <s...@samba.org> commit c3194d0d65d838b79cb5345a9d9433704b2f95ba Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Nov 17 10:23:02 2021 +1300 test/bad_chars: ensure our tests could fail Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Ralph Boehme <s...@samba.org> commit fccb105e079df7bfe22b6887262128ab9e81064d Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Tue Nov 16 20:23:04 2021 +0000 pytests: check that we don't have bad format characters Unicode has format control characters that affect the appearance — including the apparent order — of other characters. Some of these, like the bidi controls (for mixing left-to-right scripts with right-to-left scripts) can be used make text that means one thing look very much like it means another thing. The potential for duplicity using these characters has recently been publicised under the name “Trojan Source”, and CVE-2021-42694. A specific example, as it affects the Rust language is CVE-2021-42574. We don't have many format control characters in our code — in fact, just the non-breaking space (\u200b) and the redundant BOM thing (\ufeff), and this test aims to ensure we keep it that way. The test uses a series of allow-lists and deny-lists to check most text files for unknown format control characters. The filtering is fairly conservative but not exhaustive. For example, XML and text files are checked, but UTF-16 files are not. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Ralph Boehme <s...@samba.org> ----------------------------------------------------------------------- Summary of changes: lib/replace/timegm.c | 2 +- python/samba/tests/source_chars.py | 261 ++++++++++++++++++++++++++++ selftest/tests.py | 2 + source3/modules/vfs_acl_common.h | 2 +- source4/auth/gensec/gensec_krb5_heimdal.c | 2 +- testdata/source-chars-bad.c | 23 +++ testprogs/blackbox/test_samba-tool_ntacl.sh | 2 +- 7 files changed, 290 insertions(+), 4 deletions(-) create mode 100644 python/samba/tests/source_chars.py create mode 100644 testdata/source-chars-bad.c Changeset truncated at 500 lines: diff --git a/lib/replace/timegm.c b/lib/replace/timegm.c index 395c684e117..93263a2d182 100644 --- a/lib/replace/timegm.c +++ b/lib/replace/timegm.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997 Kungliga Tekniska H�gskolan + * Copyright (c) 1997 Kungliga Tekniska Högskolan * (Royal Institute of Technology, Stockholm, Sweden). * All rights reserved. * diff --git a/python/samba/tests/source_chars.py b/python/samba/tests/source_chars.py new file mode 100644 index 00000000000..f60dc5899af --- /dev/null +++ b/python/samba/tests/source_chars.py @@ -0,0 +1,261 @@ +# Unix SMB/CIFS implementation. +# +# Copyright (C) Catalyst.Net Ltd. 2021 +# +# 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/>. + + +import subprocess +import os +import sys +from collections import Counter +from samba.colour import c_RED, c_GREEN, c_DARK_YELLOW, switch_colour_off +import re +import unicodedata as u +from samba.tests import TestCase + +if not sys.stdout.isatty(): + switch_colour_off() + + +def _find_root(): + try: + p = subprocess.run(['git', 'rev-parse', '--show-toplevel'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=10) + except subprocess.SubprocessError as e: + print(c_RED(f"Error running git (is this a git tree?): {e}")) + print("This test is only useful in a git working tree") + sys.exit(1) + + root = p.stdout.decode().strip() + + should_be_roots = ( + os.path.abspath(os.path.join(os.path.dirname(__file__), + "../../..")), + os.path.abspath(os.path.join(os.path.dirname(__file__), + "../../../..")), + ) + if root not in should_be_roots: + print(c_RED("It looks like we have found the wrong git tree!")) + sys.exit(1) + return root + + +ROOT = _find_root() + + +IGNORED_FILES = { + 'examples/validchars/validchr.com', + 'examples/tridge/smb.conf', + 'source3/selftest/ktest-krb5_ccache-2', + 'source3/selftest/ktest-krb5_ccache-3', + 'third_party/pep8/testsuite/latin-1.py', + 'testdata/source-chars-bad.c', +} + +IGNORED_EXTENSIONS = { + 'cer', + 'corrupt', + 'crl', + 'dat', + 'dump', + 'gpg', + 'gz', + 'ico', + 'keytab', + 'ldb', + 'p12', + 'pem', + 'png', + 'SAMBABACKUP', + 'sxd', + 'tdb', + 'tif', + 'reg', +} + + +# This list is by no means exhaustive -- these are just the format +# characters we actually use. +SAFE_FORMAT_CHARS = { + '\u200b', + '\ufeff' +} + + +def get_git_files(): + try: + p = subprocess.run(['git', + '-C', ROOT, + 'ls-files', + '-z'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=10) + except subprocess.SubprocessError as e: + print(c_RED(f"Error running git (is this a git tree?): {e}")) + print("This test is only useful in a git working tree") + return [] + + filenames = p.stdout.split(b'\x00') + return [x.decode() for x in filenames[:-1]] + + +def iter_source_files(): + filenames = get_git_files() + + for name in filenames: + if name in IGNORED_FILES: + print(c_DARK_YELLOW(f"ignoring {name}")) + continue + + if '.' in name: + ext = name.rsplit('.', 1)[1] + if ext in IGNORED_EXTENSIONS: + print(c_DARK_YELLOW(f"ignoring {name}")) + continue + + yield name + + +def is_latin1_file(name): + for pattern in ( + r'^source4/setup/ad-schema/\w+.ldf$', + r'^source4/setup/display-specifiers/D[\w-]+.txt$', + r'^third_party/pep8/testsuite/latin-1.py$', + r'^source4/heimdal/HEIMDAL-LICENCE.txt$', + r'^source4/heimdal/lib/asn1/asn1-template.h$', + r'^source4/heimdal/lib/asn1/gen_template.c$', + r'^source4/heimdal/lib/hdb/hdb-keytab.c$', + ): + if re.match(pattern, name): + return True + return False + + +def is_bad_latin1_file(fullname): + # In practice, the few latin-1 files we have have single non-ASCII + # byte islands in a sea of ASCII. The utf-8 sequences we are + # concerned about involve sequences of 3 high bytes. We can say a + # file is safe latin-1 if it has only individual high bytes. + with open(fullname, 'rb') as f: + b = f.read() + in_seq = False + for c in b: + if c > 0x7f: + if in_seq: + return True + in_seq = True + else: + in_seq = False + return False + + +def is_bad_char(c): + if u.category(c) != 'Cf': + return False + if c in SAFE_FORMAT_CHARS: + return False + return True + + +class CharacterTests(TestCase): + def test_no_unexpected_format_chars(self): + """This test tries to ensure that no source file has unicode control + characters that can change the apparent order of other + characters. These characters could make code appear to have + different semantic meaning it really does. + + This issue is sometimes called "Trojan Source", "CVE-2021-42574", + or "CVE-2021-42694". + """ + for name in iter_source_files(): + fullname = os.path.join(ROOT, name) + try: + with open(fullname) as f: + s = f.read() + except UnicodeDecodeError as e: + # probably a latin-1 encoding, which we tolerate in a few + # files for historical reasons, though we check that there + # are not long sequences of high bytes. + if is_latin1_file(name): + if is_bad_latin1_file(fullname): + self.fail(f"latin-1 file {name} has long sequences " + "of high bytes") + else: + self.fail(f"could not decode {name}: {e}") + + for c in set(s): + if is_bad_char(c): + self.fail(f"{name} has potentially bad format characters!") + + def test_unexpected_format_chars_do_fail(self): + """Test the test""" + for name, n_bad in [ + ('testdata/source-chars-bad.c', 3) + ]: + fullname = os.path.join(ROOT, name) + with open(fullname) as f: + s = f.read() + chars = set(s) + bad_chars = [c for c in chars if is_bad_char(c)] + self.assertEqual(len(bad_chars), n_bad) + + +def check_file_text(): + """If called directly as a script, count the found characters.""" + counts = Counter() + for name in iter_source_files(): + fullname = os.path.join(ROOT, name) + try: + with open(fullname) as f: + s = f.read() + except UnicodeDecodeError as e: + if is_latin1_file(name): + if is_bad_latin1_file(fullname): + print(c_RED(f"latin-1 file {name} has long sequences " + "of high bytes")) + else: + print(c_GREEN(f"latin-1 file {name} is fine")) + else: + print(c_RED(f"can't read {name}: {e}")) + + counts.update(s) + chars = set(s) + for c in chars: + if u.category(c) == 'Cf': + print(c_GREEN(f"{name} has {u.name(c)}")) + + print(len(counts)) + controls = [] + formats = [] + others = [] + for x in counts: + c = u.category(x) + if c == 'Cc': + controls.append(x) + elif c == 'Cf': + formats.append(x) + elif c[0] == 'C': + others.append(x) + + print("normal control characters {controls}") + print("format characters {formats}") + print("other control characters {others}") + + +if __name__ == '__main__': + check_file_text() diff --git a/selftest/tests.py b/selftest/tests.py index 8ff635f2777..e92d8f54ede 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -49,6 +49,8 @@ pam_wrapper_so_path = config_hash.get("LIBPAM_WRAPPER_SO_PATH") pam_set_items_so_path = config_hash.get("PAM_SET_ITEMS_SO_PATH") planpythontestsuite("none", "samba.tests.source") +planpythontestsuite("none", "samba.tests.source_chars") + if have_man_pages_support: planpythontestsuite("none", "samba.tests.docs") diff --git a/source3/modules/vfs_acl_common.h b/source3/modules/vfs_acl_common.h index c4b4fb9c1b3..e7ec498f586 100644 --- a/source3/modules/vfs_acl_common.h +++ b/source3/modules/vfs_acl_common.h @@ -3,7 +3,7 @@ * * Copyright (C) Volker Lendecke, 2008 * Copyright (C) Jeremy Allison, 2009 - * Copyright (C) Ralph B�hme, 2016 + * Copyright (C) Ralph Böhme, 2016 * * 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 diff --git a/source4/auth/gensec/gensec_krb5_heimdal.c b/source4/auth/gensec/gensec_krb5_heimdal.c index 7e9cb5a4392..bc0d9707dc3 100644 --- a/source4/auth/gensec/gensec_krb5_heimdal.c +++ b/source4/auth/gensec/gensec_krb5_heimdal.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997 - 2006 Kungliga Tekniska H�gskolan + * Copyright (c) 1997 - 2006 Kungliga Tekniska Högskolan * (Royal Institute of Technology, Stockholm, Sweden). * All rights reserved. * diff --git a/testdata/source-chars-bad.c b/testdata/source-chars-bad.c new file mode 100644 index 00000000000..9b435722e13 --- /dev/null +++ b/testdata/source-chars-bad.c @@ -0,0 +1,23 @@ +/* + * "Trojan Source" CVE-2021-42574 test. + * + * Based on an example from https://lwn.net/Articles/874951/ + */ +#include <stdio.h> + +int main(int argc, char *argv[]) +{ + int isAdmin = 0; + +#if 0 + /* This is what is really there. */ + + /*«RLO» } «LRI»if (isAdmin)«PDI» «LRI» begin admins only */ + puts("hello admin"); + /* end admin only «RLO» { «LRI»*/ +#else + /* } if (isAdmin) begin admins only */ + puts("hello admin"); + /* end admin only { */ +#endif +} diff --git a/testprogs/blackbox/test_samba-tool_ntacl.sh b/testprogs/blackbox/test_samba-tool_ntacl.sh index 698eff8b8d2..4648fa6442f 100755 --- a/testprogs/blackbox/test_samba-tool_ntacl.sh +++ b/testprogs/blackbox/test_samba-tool_ntacl.sh @@ -1,6 +1,6 @@ #!/bin/sh # Blackbox tests for samba-tool ntacl get/set on member server -# Copyright (C) 2018 Bj�rn Baumbach <b...@sernet.de> +# Copyright (C) 2018 Björn Baumbach <b...@sernet.de> if [ $# -ne 2 ]; then echo "Usage: test_samba-tool_ntacl.sh PREFIX DOMSID" -- Samba Shared Repository