[SCM] Samba Shared Repository - branch master updated
Ralph Böhme
slow at samba.org
Wed Apr 22 09:52:05 UTC 2020
The branch, master has been updated
via fa1087bc5a9 s3: VFS: widelinks. Change call to resolve_realpath_name() -> canonicalize_absolute_path().
via 045e8898ba7 s3: selftest: Remove test_vfs_widelinks.
via e4427ff8fe6 s3: lib: Remove the old canonicalize_absolute_path().
via 594c3712c88 s3: util: Replace the old (hard to understand) canonicalize_absolute_path() with a version created from resolve_realpath_name() in vfs_widelinks.c
via d01e11cf261 s3: lib: Fix canonicalize_absolute_path() to pass the tests from resolve_realpath_name()
via 9cea0cc5b5f s3: torture: Add the tests from resolve_realpath_name() to canonicalize_absolute_path().
from b7fb6100b4c util: Fix signed/unsigned integer comparison
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit fa1087bc5a95c7743b85ee974a0bc276057d2068
Author: Jeremy Allison <jra at samba.org>
Date: Tue Apr 21 13:39:10 2020 -0700
s3: VFS: widelinks. Change call to resolve_realpath_name() -> canonicalize_absolute_path().
That code was moved into source3/lib/util_path.c.
We now have *one* canonicalize_absolute_path() funtion,
tested more completely.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
Autobuild-User(master): Ralph Böhme <slow at samba.org>
Autobuild-Date(master): Wed Apr 22 09:51:08 UTC 2020 on sn-devel-184
commit 045e8898ba727f0c7e9c39965c7d60d4692889aa
Author: Jeremy Allison <jra at samba.org>
Date: Tue Apr 21 13:34:52 2020 -0700
s3: selftest: Remove test_vfs_widelinks.
All of the tests that were in there
are now tested in samba3.smbtorture_s3.LOCAL-CANONICALIZE-PATH
along with other paths.
Clean revert of f7fe3474298 not possible due to
changes in source3/selftest/tests.py
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit e4427ff8fe66c8c371f711ec8fc59811e7383af5
Author: Jeremy Allison <jra at samba.org>
Date: Tue Apr 21 13:30:38 2020 -0700
s3: lib: Remove the old canonicalize_absolute_path().
This code was really hard to understand.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 594c3712c885447c096df9892da8f6f28f1ce8d9
Author: Jeremy Allison <jra at samba.org>
Date: Tue Apr 21 13:24:44 2020 -0700
s3: util: Replace the old (hard to understand) canonicalize_absolute_path() with a version created from resolve_realpath_name() in vfs_widelinks.c
This code is *much* more comprehensible and passes the
stricter test set than the original (unfixed) canonicalize_absolute_path()
out of the gate.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit d01e11cf261003ecf332765538216cff7001163d
Author: Jeremy Allison <jra at samba.org>
Date: Tue Apr 21 12:58:02 2020 -0700
s3: lib: Fix canonicalize_absolute_path() to pass the tests from resolve_realpath_name()
Remove the knownfail.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit 9cea0cc5b5f37e8ec612ce3a33705ffb4670a070
Author: Jeremy Allison <jra at samba.org>
Date: Tue Apr 21 11:49:44 2020 -0700
s3: torture: Add the tests from resolve_realpath_name() to canonicalize_absolute_path().
canonicalize_absolute_path() has a bug.
In canonicalize_absolute_path()
///a/./././///component/../////path/ -> /a//path
It should go to /a/path. Mark as knownfail.
Adding these tests so I can ultimately remove
resolve_realpath_name() and re-use the existing
canonicalize_absolute_path() code in vfs_widelinks.c
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/lib/util_path.c | 175 +++++++++++++++--------------------
source3/modules/test_vfs_widelinks.c | 88 ------------------
source3/modules/vfs_widelinks.c | 111 +---------------------
source3/modules/wscript_build | 5 -
source3/selftest/tests.py | 4 -
source3/torture/torture.c | 38 ++++++++
6 files changed, 116 insertions(+), 305 deletions(-)
delete mode 100644 source3/modules/test_vfs_widelinks.c
Changeset truncated at 500 lines:
diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c
index 63d01b81000..c34b734384c 100644
--- a/source3/lib/util_path.c
+++ b/source3/lib/util_path.c
@@ -107,135 +107,106 @@ char *cache_path(TALLOC_CTX *mem_ctx, const char *name)
* @retval Pointer to a talloc'ed string containing the absolute full path.
**/
-char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path)
+char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *pathname_in)
{
- char *destname;
- char *d;
- const char *s = abs_path;
- bool start_of_name_component = true;
-
- /* Allocate for strlen + '\0' + possible leading '/' */
- destname = (char *)talloc_size(ctx, strlen(abs_path) + 2);
- if (destname == NULL) {
+ /*
+ * Note we use +2 here so if pathname_in=="" then we
+ * have space to return "/".
+ */
+ char *pathname = talloc_array(ctx, char, strlen(pathname_in)+2);
+ const char *s = pathname_in;
+ char *p = pathname;
+ bool wrote_slash = false;
+
+ if (pathname == NULL) {
return NULL;
- }
- d = destname;
+ }
- *d++ = '/'; /* Always start with root. */
+ /* Always start with a '/'. */
+ *p++ = '/';
+ wrote_slash = true;
while (*s) {
- if (*s == '/') {
- /* Eat multiple '/' */
- while (*s == '/') {
+ /* Deal with '/' or multiples of '/'. */
+ if (s[0] == '/') {
+ while (s[0] == '/') {
+ /* Eat trailing '/' */
s++;
}
- if ((d > destname + 1) && (*s != '\0')) {
- *d++ = '/';
+ /* Update target with one '/' */
+ if (!wrote_slash) {
+ *p++ = '/';
+ wrote_slash = true;
}
- start_of_name_component = true;
continue;
}
-
- if (start_of_name_component) {
- if ((s[0] == '.') && (s[1] == '.') &&
+ if (wrote_slash) {
+ /* Deal with "./" or ".\0" */
+ if (s[0] == '.' &&
+ (s[1] == '/' || s[1] == '\0')) {
+ /* Eat the dot. */
+ s++;
+ while (s[0] == '/') {
+ /* Eat any trailing '/' */
+ s++;
+ }
+ /* Don't write anything to target. */
+ /* wrote_slash is still true. */
+ continue;
+ }
+ /* Deal with "../" or "..\0" */
+ if (s[0] == '.' && s[1] == '.' &&
(s[2] == '/' || s[2] == '\0')) {
- /* Uh oh - "/../" or "/..\0" ! */
-
- /* Go past the .. leaving us on the / or '\0' */
+ /* Eat the dot dot. */
s += 2;
-
- /* If we just added a '/' - delete it */
- if ((d > destname) && (*(d-1) == '/')) {
- *(d-1) = '\0';
- d--;
+ while (s[0] == '/') {
+ /* Eat any trailing '/' */
+ s++;
}
-
/*
- * Are we at the start ?
- * Can't go back further if so.
+ * As wrote_slash is true, we go back
+ * one character to point p at the slash
+ * we just saw.
*/
- if (d <= destname) {
- *d++ = '/'; /* Can't delete root */
- continue;
+ if (p > pathname) {
+ p--;
}
- /* Go back one level... */
/*
- * Decrement d first as d points to
- * the *next* char to write into.
+ * Now go back to the slash
+ * before the one that p currently points to.
*/
- for (d--; d > destname; d--) {
- if (*d == '/') {
+ while (p > pathname) {
+ p--;
+ if (p[0] == '/') {
break;
}
}
-
/*
- * Are we at the start ?
- * Can't go back further if so.
+ * Step forward one to leave the
+ * last written '/' alone.
*/
- if (d <= destname) {
- *d++ = '/'; /* Can't delete root */
- continue;
- }
+ p++;
- /*
- * We're still at the start of a name
- * component, just the previous one.
- */
- continue;
- } else if ((s[0] == '.') &&
- ((s[1] == '\0') || s[1] == '/')) {
- /*
- * Component of pathname can't be "." only.
- * Skip the '.' .
- */
- if (s[1] == '/') {
- s += 2;
- } else {
- s++;
- }
+ /* Don't write anything to target. */
+ /* wrote_slash is still true. */
continue;
}
}
-
- if (!(*s & 0x80)) {
- *d++ = *s++;
- } else {
- size_t siz;
- /* Get the size of the next MB character. */
- next_codepoint(s,&siz);
- switch(siz) {
- case 5:
- *d++ = *s++;
-
- FALL_THROUGH;
- case 4:
- *d++ = *s++;
-
- FALL_THROUGH;
- case 3:
- *d++ = *s++;
-
- FALL_THROUGH;
- case 2:
- *d++ = *s++;
-
- FALL_THROUGH;
- case 1:
- *d++ = *s++;
- break;
- default:
- break;
- }
- }
- start_of_name_component = false;
+ /* Non-separator character, just copy. */
+ *p++ = *s++;
+ wrote_slash = false;
}
- *d = '\0';
-
- /* And must not end in '/' */
- if (d > destname + 1 && (*(d-1) == '/')) {
- *(d-1) = '\0';
+ if (wrote_slash) {
+ /*
+ * We finished on a '/'.
+ * Remove the trailing '/', but not if it's
+ * the sole character in the path.
+ */
+ if (p > pathname + 1) {
+ p--;
+ }
}
-
- return destname;
+ /* Terminate and we're done ! */
+ *p++ = '\0';
+ return pathname;
}
diff --git a/source3/modules/test_vfs_widelinks.c b/source3/modules/test_vfs_widelinks.c
deleted file mode 100644
index c6055c8c17c..00000000000
--- a/source3/modules/test_vfs_widelinks.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * Unix SMB/CIFS implementation.
- *
- * Unit test for widelinks path validator.
- *
- * Copyright (C) Jeremy Allison 2020
- *
- * 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/>.
- */
-
-/* Needed for static build to complete... */
-#include "includes.h"
-#include "smbd/smbd.h"
-NTSTATUS vfs_widelinks_init(TALLOC_CTX *ctx);
-
-#include "vfs_widelinks.c"
-#include <cmocka.h>
-
-struct str_test_values {
- const char *src_str;
- const char *dst_str;
-} ;
-
-/* As many nasty edge cases as I can think of.. */
-
-static struct str_test_values examples[] = {
- { "/", "/" },
- { "/../../", "/" },
- { "/foo/../", "/" },
- { "/./././", "/" },
- { "/./././.", "/" },
- { "/.../././.", "/..." },
- { "/./././.foo", "/.foo" },
- { "/./././.foo.", "/.foo." },
- { "/./././foo.", "/foo." },
- { "/foo/bar/..", "/foo" },
- { "/foo/bar/../baz/", "/foo/baz" },
- { "////////////////", "/" },
- { "/////////./././././.", "/" },
- { "/./.././../.boo/../baz", "/baz" },
- { "/a/component/path", "/a/component/path" },
- { "/a/component/path/", "/a/component/path" },
- { "/a/component/path/..", "/a/component" },
- { "/a/component/../path/", "/a/path" },
- { "///a/./././///component/../////path/", "/a/path" }
-};
-
-/*
- * Test our realpath resolution code.
- */
-static void test_resolve_realpath_name(void **state)
-{
- unsigned i;
- TALLOC_CTX *frame = talloc_stackframe();
-
- for (i = 0; i < ARRAY_SIZE(examples); i++) {
- char *test_dst = resolve_realpath_name(frame,
- examples[i].src_str);
- if (test_dst == NULL) {
- fail();
- }
- assert_string_equal(test_dst, examples[i].dst_str);
- TALLOC_FREE(test_dst);
- }
- TALLOC_FREE(frame);
-}
-
-int main(int argc, char **argv)
-{
- const struct CMUnitTest tests[] = {
- cmocka_unit_test(test_resolve_realpath_name),
- };
-
- cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
-
- return cmocka_run_group_tests(tests, NULL, NULL);
-}
diff --git a/source3/modules/vfs_widelinks.c b/source3/modules/vfs_widelinks.c
index 9a005c9ce3f..36bd3ec6377 100644
--- a/source3/modules/vfs_widelinks.c
+++ b/source3/modules/vfs_widelinks.c
@@ -102,117 +102,13 @@
#include "includes.h"
#include "smbd/smbd.h"
+#include "lib/util_path.h"
struct widelinks_config {
bool active;
char *cwd;
};
-/*
- * Canonicalizes an absolute path, removing '.' and ".." components
- * and consolitating multiple '/' characters. As this can only be
- * called once widelinks_chdir with an absolute path has been called,
- * we can assert that the start of path must be '/'.
- */
-
-static char *resolve_realpath_name(TALLOC_CTX *ctx, const char *pathname_in)
-{
- const char *s = pathname_in;
- char *pathname = talloc_array(ctx, char, strlen(pathname_in)+1);
- char *p = pathname;
- bool wrote_slash = false;
-
- if (pathname == NULL) {
- return NULL;
- }
-
- SMB_ASSERT(pathname_in[0] == '/');
-
- while (*s) {
- /* Deal with '/' or multiples of '/'. */
- if (s[0] == '/') {
- while (s[0] == '/') {
- /* Eat trailing '/' */
- s++;
- }
- /* Update target with one '/' */
- if (!wrote_slash) {
- *p++ = '/';
- wrote_slash = true;
- }
- continue;
- }
- if (wrote_slash) {
- /* Deal with "./" or ".\0" */
- if (s[0] == '.' &&
- (s[1] == '/' || s[1] == '\0')) {
- /* Eat the dot. */
- s++;
- while (s[0] == '/') {
- /* Eat any trailing '/' */
- s++;
- }
- /* Don't write anything to target. */
- /* wrote_slash is still true. */
- continue;
- }
- /* Deal with "../" or "..\0" */
- if (s[0] == '.' && s[1] == '.' &&
- (s[2] == '/' || s[2] == '\0')) {
- /* Eat the dot dot. */
- s += 2;
- while (s[0] == '/') {
- /* Eat any trailing '/' */
- s++;
- }
- /*
- * As wrote_slash is true, we go back
- * one character to point p at the slash
- * we just saw.
- */
- if (p > pathname) {
- p--;
- }
- /*
- * Now go back to the slash
- * before the one that p currently points to.
- */
- while (p > pathname) {
- p--;
- if (p[0] == '/') {
- break;
- }
- }
- /*
- * Step forward one to leave the
- * last written '/' alone.
- */
- p++;
-
- /* Don't write anything to target. */
- /* wrote_slash is still true. */
- continue;
- }
- }
- /* Non-separator character, just copy. */
- *p++ = *s++;
- wrote_slash = false;
- }
- if (wrote_slash) {
- /*
- * We finished on a '/'.
- * Remove the trailing '/', but not if it's
- * the sole character in the path.
- */
- if (p > pathname + 1) {
- p--;
- }
- }
- /* Terminate and we're done ! */
- *p++ = '\0';
- return pathname;
-}
-
static int widelinks_connect(struct vfs_handle_struct *handle,
const char *service,
const char *user)
@@ -384,7 +280,10 @@ static struct smb_filename *widelinks_realpath(vfs_handle_struct *handle,
config->cwd,
smb_fname_in->base_name);
}
- resolved_pathname = resolve_realpath_name(config, pathname);
+
+ SMB_ASSERT(pathname[0] == '/');
+
+ resolved_pathname = canonicalize_absolute_path(config, pathname);
if (resolved_pathname == NULL) {
TALLOC_FREE(pathname);
return NULL;
diff --git a/source3/modules/wscript_build b/source3/modules/wscript_build
index 1f2d2d3acb6..7f056f2b7f7 100644
--- a/source3/modules/wscript_build
+++ b/source3/modules/wscript_build
@@ -628,8 +628,3 @@ bld.SAMBA3_MODULE('vfs_widelinks',
init_function='',
internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_widelinks'),
enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_widelinks'))
-
-bld.SAMBA3_BINARY('test_vfs_widelinks',
- source='test_vfs_widelinks.c',
- deps='smbd_base cmocka',
- for_selftest=True)
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index a536a473cb5..8210e981c35 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -508,10 +508,6 @@ plantestsuite("samba3.test_nfs4_acl", "none",
[os.path.join(bindir(), "test_nfs4_acls"),
"$SMB_CONF_PATH"])
-plantestsuite("samba3.test_vfs_widelinks", "none",
- [os.path.join(bindir(), "test_vfs_widelinks"),
- "$SMB_CONF_PATH"])
-
plantestsuite("samba3.test_vfs_full_audit", "none",
[os.path.join(bindir(), "test_vfs_full_audit"),
"$SMB_CONF_PATH"])
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index c3f387cd0e2..aa02163636e 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -14066,6 +14066,25 @@ static bool run_local_canonicalize_path(int dummy)
".././././",
".././././../../../boo",
"./..",
+ "/",
+ "/../../",
+ "/foo/../",
+ "/./././",
+ "/./././.",
+ "/.../././.",
+ "/./././.foo",
+ "/./././.foo.",
+ "/./././foo.",
+ "/foo/bar/..",
+ "/foo/bar/../baz/",
+ "////////////////",
+ "/////////./././././.",
+ "/./.././../.boo/../baz",
+ "/a/component/path",
+ "/a/component/path/",
+ "/a/component/path/..",
+ "/a/component/../path/",
+ "///a/./././///component/../////path/",
NULL
};
const char *dst[] = {
--
Samba Shared Repository
More information about the samba-cvs
mailing list