From f19754cfa6be8fde5012a77f1c8cc77500a74018 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 13:41:24 -0700 Subject: [PATCH] GUACAMOLE-637: Add unit tests for SFTP path normalization. --- configure.ac | 1 + src/common-ssh/.gitignore | 5 + src/common-ssh/Makefile.am | 1 + src/common-ssh/common-ssh/sftp.h | 25 +++ src/common-ssh/sftp.c | 24 +-- src/common-ssh/tests/Makefile.am | 66 +++++++ src/common-ssh/tests/sftp/normalize_path.c | 216 +++++++++++++++++++++ 7 files changed, 315 insertions(+), 23 deletions(-) create mode 100644 src/common-ssh/.gitignore create mode 100644 src/common-ssh/tests/Makefile.am create mode 100644 src/common-ssh/tests/sftp/normalize_path.c diff --git a/configure.ac b/configure.ac index 7ea60298..13231239 100644 --- a/configure.ac +++ b/configure.ac @@ -1317,6 +1317,7 @@ AC_CONFIG_FILES([Makefile src/common/Makefile src/common/tests/Makefile src/common-ssh/Makefile + src/common-ssh/tests/Makefile src/terminal/Makefile src/libguac/Makefile src/libguac/tests/Makefile diff --git a/src/common-ssh/.gitignore b/src/common-ssh/.gitignore new file mode 100644 index 00000000..f18cde53 --- /dev/null +++ b/src/common-ssh/.gitignore @@ -0,0 +1,5 @@ + +# Auto-generated test runner and binary +_generated_runner.c +test_common_ssh + diff --git a/src/common-ssh/Makefile.am b/src/common-ssh/Makefile.am index 81167238..8402e5b0 100644 --- a/src/common-ssh/Makefile.am +++ b/src/common-ssh/Makefile.am @@ -27,6 +27,7 @@ AUTOMAKE_OPTIONS = foreign ACLOCAL_AMFLAGS = -I m4 noinst_LTLIBRARIES = libguac_common_ssh.la +SUBDIRS = . tests libguac_common_ssh_la_SOURCES = \ buffer.c \ diff --git a/src/common-ssh/common-ssh/sftp.h b/src/common-ssh/common-ssh/sftp.h index 0ec2d12b..eafdf21f 100644 --- a/src/common-ssh/common-ssh/sftp.h +++ b/src/common-ssh/common-ssh/sftp.h @@ -255,5 +255,30 @@ int guac_common_ssh_sftp_handle_file_stream( void guac_common_ssh_sftp_set_upload_path( guac_common_ssh_sftp_filesystem* filesystem, const char* path); +/** + * Given an arbitrary absolute path, which may contain "..", ".", and + * backslashes, creates an equivalent absolute path which does NOT contain + * relative path components (".." or "."), backslashes, or empty path + * components. With the exception of paths referring to the root directory, the + * resulting path is guaranteed to not contain trailing slashes. + * + * Normalization will fail if the given path is not absolute, is too long, or + * contains more than GUAC_COMMON_SSH_SFTP_MAX_DEPTH path components. + * + * @param fullpath + * The buffer to populate with the normalized path. The normalized path + * will not contain relative path components like ".." or ".", nor will it + * contain backslashes. This buffer MUST be at least + * GUAC_COMMON_SSH_SFTP_MAX_PATH bytes in size. + * + * @param path + * The absolute path to normalize. + * + * @return + * Non-zero if normalization succeeded, zero otherwise. + */ +int guac_common_ssh_sftp_normalize_path(char* fullpath, + const char* path); + #endif diff --git a/src/common-ssh/sftp.c b/src/common-ssh/sftp.c index 0a24c049..786ccc8d 100644 --- a/src/common-ssh/sftp.c +++ b/src/common-ssh/sftp.c @@ -33,29 +33,7 @@ #include #include -/** - * Given an arbitrary absolute path, which may contain "..", ".", and - * backslashes, creates an equivalent absolute path which does NOT contain - * relative path components (".." or "."), backslashes, or empty path - * components. With the exception of paths referring to the root directory, the - * resulting path is guaranteed to not contain trailing slashes. - * - * Normalization will fail if the given path is not absolute, is too long, or - * contains more than GUAC_COMMON_SSH_SFTP_MAX_DEPTH path components. - * - * @param fullpath - * The buffer to populate with the normalized path. The normalized path - * will not contain relative path components like ".." or ".", nor will it - * contain backslashes. This buffer MUST be at least - * GUAC_COMMON_SSH_SFTP_MAX_PATH bytes in size. - * - * @param path - * The absolute path to normalize. - * - * @return - * Non-zero if normalization succeeded, zero otherwise. - */ -static int guac_common_ssh_sftp_normalize_path(char* fullpath, +int guac_common_ssh_sftp_normalize_path(char* fullpath, const char* path) { int i; diff --git a/src/common-ssh/tests/Makefile.am b/src/common-ssh/tests/Makefile.am new file mode 100644 index 00000000..b26d5bbf --- /dev/null +++ b/src/common-ssh/tests/Makefile.am @@ -0,0 +1,66 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# NOTE: Parts of this file (Makefile.am) are automatically transcluded verbatim +# into Makefile.in. Though the build system (GNU Autotools) automatically adds +# its own license boilerplate to the generated Makefile.in, that boilerplate +# does not apply to the transcluded portions of Makefile.am which are licensed +# to you by the ASF under the Apache License, Version 2.0, as described above. +# + +AUTOMAKE_OPTIONS = foreign +ACLOCAL_AMFLAGS = -I m4 + +# +# Unit tests for common SSH support +# + +check_PROGRAMS = test_common_ssh +TESTS = $(check_PROGRAMS) + +test_common_ssh_SOURCES = \ + sftp/normalize_path.c + +test_common_ssh_CFLAGS = \ + -Werror -Wall -pedantic \ + @COMMON_INCLUDE@ \ + @COMMON_SSH_INCLUDE@ + +test_common_ssh_LDADD = \ + @CUNIT_LIBS@ \ + @COMMON_SSH_LTLIB@ \ + @COMMON_LTLIB@ + +# +# Autogenerate test runner +# + +GEN_RUNNER = $(top_srcdir)/util/generate-test-runner.pl +CLEANFILES = _generated_runner.c + +_generated_runner.c: $(test_common_ssh_SOURCES) + $(AM_V_GEN) $(GEN_RUNNER) $(test_common_ssh_SOURCES) > $@ + +nodist_test_common_ssh_SOURCES = \ + _generated_runner.c + +# Use automake's TAP test driver for running any tests +LOG_DRIVER = \ + env AM_TAP_AWK='$(AWK)' \ + $(SHELL) $(top_srcdir)/build-aux/tap-driver.sh + diff --git a/src/common-ssh/tests/sftp/normalize_path.c b/src/common-ssh/tests/sftp/normalize_path.c new file mode 100644 index 00000000..23975252 --- /dev/null +++ b/src/common-ssh/tests/sftp/normalize_path.c @@ -0,0 +1,216 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "common-ssh/sftp.h" + +#include +#include + +/** + * Test which verifies absolute Windows-style paths are correctly normalized to + * absolute paths with UNIX separators and no relative components. + */ +void test_fs__normalize_absolute_windows() { + + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo\\bar\\baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/bar/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo\\bar\\..\\baz\\"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo\\bar\\..\\..\\baz\\a\\..\\b"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/baz/b", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo\\.\\bar\\baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/bar/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo\\bar\\..\\..\\..\\..\\..\\..\\baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/baz", sizeof(normalized)); + +} + +/** + * Test which verifies absolute UNIX-style paths are correctly normalized to + * absolute paths with UNIX separators and no relative components. + */ +void test_fs__normalize_absolute_unix() { + + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/foo/bar/baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/bar/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/foo/bar/../baz/"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/foo/bar/../../baz/a/../b"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/baz/b", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/foo/./bar/baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/bar/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/foo/bar/../../../../../../baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/baz", sizeof(normalized)); + +} + +/** + * Test which verifies absolute paths consisting of mixed Windows and UNIX path + * separators are correctly normalized to absolute paths with UNIX separators + * and no relative components. + */ +void test_fs__normalize_absolute_mixed() { + + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo/bar\\baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/bar/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "/foo\\bar/..\\baz/"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo/bar\\../../baz\\a\\..\\b"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/baz/b", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo\\.\\bar/baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/foo/bar/baz", sizeof(normalized)); + + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "\\foo/bar\\../..\\..\\..\\../..\\baz"), 0); + CU_ASSERT_NSTRING_EQUAL(normalized, "/baz", sizeof(normalized)); + +} + +/** + * Test which verifies relative Windows-style paths are always rejected. + */ +void test_fs__normalize_relative_windows() { + + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ""), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "."), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ".."), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "foo"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ".\\foo"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "..\\foo"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "foo\\bar\\baz"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ".\\foo\\bar\\baz"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "..\\foo\\bar\\baz"), 0); + +} + +/** + * Test which verifies relative UNIX-style paths are always rejected. + */ +void test_fs__normalize_relative_unix() { + + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ""), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "."), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ".."), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "foo"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "./foo"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "../foo"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "foo/bar/baz"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "./foo/bar/baz"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "../foo/bar/baz"), 0); + +} + +/** + * Test which verifies relative paths consisting of mixed Windows and UNIX path + * separators are always rejected. + */ +void test_fs__normalize_relative_mixed() { + + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "foo\\bar/baz"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, ".\\foo/bar/baz"), 0); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, "../foo\\bar\\baz"), 0); + +} + +/** + * Generates a dynamically-allocated path having the given number of bytes, not + * counting the null-terminator. The path will contain only UNIX-style path + * separators. The returned path must eventually be freed with a call to + * free(). + * + * @param length + * The number of bytes to include in the generated path, not counting the + * null-terminator. + * + * @return + * A dynamically-allocated path containing the given number of bytes, not + * counting the null-terminator. This path must eventually be freed with a + * call to free(). + */ +static char* generate_path(int length) { + + int i; + char* input = malloc(length + 1); + + /* Fill path with /x/x/x/x/x/x/x/x/x/x/... */ + for (i = 0; i < length; i++) { + input[i] = (i % 2 == 0) ? '/' : 'x'; + } + + /* Add null terminator */ + input[length] = '\0'; + + return input; + +} + +/** + * Test which verifies that paths exceeding the maximum path length are + * rejected. + */ +void test_fs__normalize_long() { + + char* input; + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + /* Exceeds maximum length by a factor of 2 */ + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH*2); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + + /* Exceeds maximum length by one byte */ + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + + /* Exactly maximum length */ + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH - 1); + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + +} +