GUACAMOLE-637: Add unit tests for SFTP path normalization.

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

src/common-ssh/.gitignore

@@ -0,0 +1,5 @@
+
+# Auto-generated test runner and binary
+_generated_runner.c
+test_common_ssh
+

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                    \

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
 

src/common-ssh/sftp.c

@@ -33,29 +33,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-/**
+int guac_common_ssh_sftp_normalize_path(char* fullpath,
- * 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,
         const char* path) {
 
     int i;

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
+

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 <CUnit/CUnit.h>
+#include <stdlib.h>
+
+/**
+ * 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);
+
+}
+