From f6953e131764284915e51bd495a510c7c592d59b Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 19 Oct 2018 09:30:20 -0700 Subject: [PATCH 01/26] GUACAMOLE-637: Use proper namespaced path for Guacamole headers within libguac source. --- src/libguac/Makefile.am | 2 +- src/libguac/audio.c | 11 +++++------ src/libguac/client.c | 20 ++++++++++---------- src/libguac/encode-jpeg.c | 6 +++--- src/libguac/encode-jpeg.h | 4 ++-- src/libguac/encode-png.c | 6 +++--- src/libguac/encode-png.h | 4 ++-- src/libguac/encode-webp.c | 6 +++--- src/libguac/encode-webp.h | 4 ++-- src/libguac/error.c | 2 +- src/libguac/id.c | 2 +- src/libguac/parser.c | 8 ++++---- src/libguac/pool.c | 2 +- src/libguac/protocol.c | 14 +++++++------- src/libguac/raw_encoder.c | 12 +++++------- src/libguac/raw_encoder.h | 2 +- src/libguac/socket-broadcast.c | 8 ++++---- src/libguac/socket-fd.c | 4 ++-- src/libguac/socket-nest.c | 6 +++--- src/libguac/socket-ssl.c | 6 +++--- src/libguac/socket-tee.c | 2 +- src/libguac/socket-wsa.c | 4 ++-- src/libguac/socket.c | 8 ++++---- src/libguac/timestamp.c | 2 +- src/libguac/unicode.c | 2 +- src/libguac/user-handlers.c | 12 ++++++------ src/libguac/user-handlers.h | 4 ++-- src/libguac/user-handshake.c | 12 ++++++------ src/libguac/user.c | 16 ++++++++-------- 29 files changed, 94 insertions(+), 97 deletions(-) diff --git a/src/libguac/Makefile.am b/src/libguac/Makefile.am index 2757f706..876eb298 100644 --- a/src/libguac/Makefile.am +++ b/src/libguac/Makefile.am @@ -122,7 +122,7 @@ libguacinc_HEADERS += guacamole/socket-wsa.h endif libguac_la_CFLAGS = \ - -Werror -Wall -pedantic -I$(srcdir)/guacamole + -Werror -Wall -pedantic libguac_la_LDFLAGS = \ -version-info 16:0:0 \ diff --git a/src/libguac/audio.c b/src/libguac/audio.c index cf0187a2..cc577c71 100644 --- a/src/libguac/audio.c +++ b/src/libguac/audio.c @@ -19,14 +19,13 @@ #include "config.h" +#include "guacamole/audio.h" +#include "guacamole/client.h" +#include "guacamole/protocol.h" +#include "guacamole/stream.h" +#include "guacamole/user.h" #include "raw_encoder.h" -#include -#include -#include -#include -#include - #include #include diff --git a/src/libguac/client.c b/src/libguac/client.c index 4f3051d9..71cffda8 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -19,20 +19,20 @@ #include "config.h" -#include "client.h" #include "encode-jpeg.h" #include "encode-png.h" #include "encode-webp.h" -#include "error.h" +#include "guacamole/client.h" +#include "guacamole/error.h" +#include "guacamole/layer.h" +#include "guacamole/plugin.h" +#include "guacamole/pool.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/stream.h" +#include "guacamole/timestamp.h" +#include "guacamole/user.h" #include "id.h" -#include "layer.h" -#include "pool.h" -#include "plugin.h" -#include "protocol.h" -#include "socket.h" -#include "stream.h" -#include "timestamp.h" -#include "user.h" #include #include diff --git a/src/libguac/encode-jpeg.c b/src/libguac/encode-jpeg.c index 5a869c72..1aeb23bb 100644 --- a/src/libguac/encode-jpeg.c +++ b/src/libguac/encode-jpeg.c @@ -20,10 +20,10 @@ #include "config.h" #include "encode-jpeg.h" -#include "error.h" +#include "guacamole/error.h" +#include "guacamole/protocol.h" +#include "guacamole/stream.h" #include "palette.h" -#include "protocol.h" -#include "stream.h" #include #include diff --git a/src/libguac/encode-jpeg.h b/src/libguac/encode-jpeg.h index d791ea69..05cf2cad 100644 --- a/src/libguac/encode-jpeg.h +++ b/src/libguac/encode-jpeg.h @@ -22,8 +22,8 @@ #include "config.h" -#include "socket.h" -#include "stream.h" +#include "guacamole/socket.h" +#include "guacamole/stream.h" #include diff --git a/src/libguac/encode-png.c b/src/libguac/encode-png.c index 78795577..7d4b6c7f 100644 --- a/src/libguac/encode-png.c +++ b/src/libguac/encode-png.c @@ -20,10 +20,10 @@ #include "config.h" #include "encode-png.h" -#include "error.h" +#include "guacamole/error.h" +#include "guacamole/protocol.h" +#include "guacamole/stream.h" #include "palette.h" -#include "protocol.h" -#include "stream.h" #include #include diff --git a/src/libguac/encode-png.h b/src/libguac/encode-png.h index 916493c4..222c50ef 100644 --- a/src/libguac/encode-png.h +++ b/src/libguac/encode-png.h @@ -22,8 +22,8 @@ #include "config.h" -#include "socket.h" -#include "stream.h" +#include "guacamole/socket.h" +#include "guacamole/stream.h" #include diff --git a/src/libguac/encode-webp.c b/src/libguac/encode-webp.c index 88534db5..5c2237d6 100644 --- a/src/libguac/encode-webp.c +++ b/src/libguac/encode-webp.c @@ -20,10 +20,10 @@ #include "config.h" #include "encode-webp.h" -#include "error.h" +#include "guacamole/error.h" +#include "guacamole/protocol.h" +#include "guacamole/stream.h" #include "palette.h" -#include "protocol.h" -#include "stream.h" #include #include diff --git a/src/libguac/encode-webp.h b/src/libguac/encode-webp.h index 8971765c..5347f6d9 100644 --- a/src/libguac/encode-webp.h +++ b/src/libguac/encode-webp.h @@ -22,8 +22,8 @@ #include "config.h" -#include "socket.h" -#include "stream.h" +#include "guacamole/socket.h" +#include "guacamole/stream.h" #include diff --git a/src/libguac/error.c b/src/libguac/error.c index 95942e81..5561b178 100644 --- a/src/libguac/error.c +++ b/src/libguac/error.c @@ -19,7 +19,7 @@ #include "config.h" -#include "error.h" +#include "guacamole/error.h" #include #include diff --git a/src/libguac/id.c b/src/libguac/id.c index 56ef23c2..27a714c0 100644 --- a/src/libguac/id.c +++ b/src/libguac/id.c @@ -19,8 +19,8 @@ #include "config.h" +#include "guacamole/error.h" #include "id.h" -#include "error.h" #ifdef HAVE_OSSP_UUID_H #include diff --git a/src/libguac/parser.c b/src/libguac/parser.c index 799b48ab..9645a2a3 100644 --- a/src/libguac/parser.c +++ b/src/libguac/parser.c @@ -19,10 +19,10 @@ #include "config.h" -#include "error.h" -#include "parser.h" -#include "socket.h" -#include "unicode.h" +#include "guacamole/error.h" +#include "guacamole/parser.h" +#include "guacamole/socket.h" +#include "guacamole/unicode.h" #include #include diff --git a/src/libguac/pool.c b/src/libguac/pool.c index 363c239d..9db43ada 100644 --- a/src/libguac/pool.c +++ b/src/libguac/pool.c @@ -19,7 +19,7 @@ #include "config.h" -#include "pool.h" +#include "guacamole/pool.h" #include diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index dba9c56e..ee266e87 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -19,14 +19,14 @@ #include "config.h" -#include "error.h" -#include "layer.h" -#include "object.h" +#include "guacamole/error.h" +#include "guacamole/layer.h" +#include "guacamole/object.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/stream.h" +#include "guacamole/unicode.h" #include "palette.h" -#include "protocol.h" -#include "socket.h" -#include "stream.h" -#include "unicode.h" #include diff --git a/src/libguac/raw_encoder.c b/src/libguac/raw_encoder.c index 1dd03cc4..9086a37b 100644 --- a/src/libguac/raw_encoder.c +++ b/src/libguac/raw_encoder.c @@ -19,15 +19,13 @@ #include "config.h" -#include "audio.h" +#include "guacamole/audio.h" +#include "guacamole/client.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/user.h" #include "raw_encoder.h" -#include -#include -#include -#include -#include - #include #include #include diff --git a/src/libguac/raw_encoder.h b/src/libguac/raw_encoder.h index b4050406..c3af151c 100644 --- a/src/libguac/raw_encoder.h +++ b/src/libguac/raw_encoder.h @@ -23,7 +23,7 @@ #include "config.h" -#include "audio.h" +#include "guacamole/audio.h" /** * The number of bytes to send in each audio blob. diff --git a/src/libguac/socket-broadcast.c b/src/libguac/socket-broadcast.c index a3f17fd0..f551e817 100644 --- a/src/libguac/socket-broadcast.c +++ b/src/libguac/socket-broadcast.c @@ -19,10 +19,10 @@ #include "config.h" -#include "client.h" -#include "error.h" -#include "socket.h" -#include "user.h" +#include "guacamole/client.h" +#include "guacamole/error.h" +#include "guacamole/socket.h" +#include "guacamole/user.h" #include #include diff --git a/src/libguac/socket-fd.c b/src/libguac/socket-fd.c index 34898524..742cc35d 100644 --- a/src/libguac/socket-fd.c +++ b/src/libguac/socket-fd.c @@ -19,8 +19,8 @@ #include "config.h" -#include "error.h" -#include "socket.h" +#include "guacamole/error.h" +#include "guacamole/socket.h" #include "wait-fd.h" #include diff --git a/src/libguac/socket-nest.c b/src/libguac/socket-nest.c index 967c9d97..8bc9291e 100644 --- a/src/libguac/socket-nest.c +++ b/src/libguac/socket-nest.c @@ -19,9 +19,9 @@ #include "config.h" -#include "protocol.h" -#include "socket.h" -#include "unicode.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/unicode.h" #include #include diff --git a/src/libguac/socket-ssl.c b/src/libguac/socket-ssl.c index 1a631fe1..3daa128e 100644 --- a/src/libguac/socket-ssl.c +++ b/src/libguac/socket-ssl.c @@ -19,9 +19,9 @@ #include "config.h" -#include "error.h" -#include "socket-ssl.h" -#include "socket.h" +#include "guacamole/error.h" +#include "guacamole/socket-ssl.h" +#include "guacamole/socket.h" #include "wait-fd.h" #include diff --git a/src/libguac/socket-tee.c b/src/libguac/socket-tee.c index 3f0fd6bd..cee9108c 100644 --- a/src/libguac/socket-tee.c +++ b/src/libguac/socket-tee.c @@ -19,7 +19,7 @@ #include "config.h" -#include "socket.h" +#include "guacamole/socket.h" #include diff --git a/src/libguac/socket-wsa.c b/src/libguac/socket-wsa.c index 1b59176a..f5602e39 100644 --- a/src/libguac/socket-wsa.c +++ b/src/libguac/socket-wsa.c @@ -17,8 +17,8 @@ * under the License. */ -#include "error.h" -#include "socket.h" +#include "guacamole/error.h" +#include "guacamole/socket.h" #include #include diff --git a/src/libguac/socket.c b/src/libguac/socket.c index 6bc0036b..66442d0c 100644 --- a/src/libguac/socket.c +++ b/src/libguac/socket.c @@ -19,10 +19,10 @@ #include "config.h" -#include "error.h" -#include "protocol.h" -#include "socket.h" -#include "timestamp.h" +#include "guacamole/error.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/timestamp.h" #include #include diff --git a/src/libguac/timestamp.c b/src/libguac/timestamp.c index 0d2dc0a8..9020a6e1 100644 --- a/src/libguac/timestamp.c +++ b/src/libguac/timestamp.c @@ -19,7 +19,7 @@ #include "config.h" -#include "timestamp.h" +#include "guacamole/timestamp.h" #include diff --git a/src/libguac/unicode.c b/src/libguac/unicode.c index e7af943f..fdc0fffd 100644 --- a/src/libguac/unicode.c +++ b/src/libguac/unicode.c @@ -19,7 +19,7 @@ #include "config.h" -#include "unicode.h" +#include "guacamole/unicode.h" #include diff --git a/src/libguac/user-handlers.c b/src/libguac/user-handlers.c index b84dc721..6be20e2e 100644 --- a/src/libguac/user-handlers.c +++ b/src/libguac/user-handlers.c @@ -19,12 +19,12 @@ #include "config.h" -#include "client.h" -#include "object.h" -#include "protocol.h" -#include "stream.h" -#include "timestamp.h" -#include "user.h" +#include "guacamole/client.h" +#include "guacamole/object.h" +#include "guacamole/protocol.h" +#include "guacamole/stream.h" +#include "guacamole/timestamp.h" +#include "guacamole/user.h" #include "user-handlers.h" #include diff --git a/src/libguac/user-handlers.h b/src/libguac/user-handlers.h index eedeba15..5d7c6eae 100644 --- a/src/libguac/user-handlers.h +++ b/src/libguac/user-handlers.h @@ -31,8 +31,8 @@ #include "config.h" -#include "client.h" -#include "timestamp.h" +#include "guacamole/client.h" +#include "guacamole/timestamp.h" /** * Internal handler for Guacamole instructions. Instruction handlers will be diff --git a/src/libguac/user-handshake.c b/src/libguac/user-handshake.c index b6018888..13bea51f 100644 --- a/src/libguac/user-handshake.c +++ b/src/libguac/user-handshake.c @@ -19,12 +19,12 @@ #include "config.h" -#include "client.h" -#include "error.h" -#include "parser.h" -#include "protocol.h" -#include "socket.h" -#include "user.h" +#include "guacamole/client.h" +#include "guacamole/error.h" +#include "guacamole/parser.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/user.h" #include #include diff --git a/src/libguac/user.c b/src/libguac/user.c index 14ec75bf..16590060 100644 --- a/src/libguac/user.c +++ b/src/libguac/user.c @@ -19,18 +19,18 @@ #include "config.h" -#include "client.h" #include "encode-jpeg.h" #include "encode-png.h" #include "encode-webp.h" +#include "guacamole/client.h" +#include "guacamole/object.h" +#include "guacamole/pool.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" +#include "guacamole/stream.h" +#include "guacamole/timestamp.h" +#include "guacamole/user.h" #include "id.h" -#include "object.h" -#include "pool.h" -#include "protocol.h" -#include "socket.h" -#include "stream.h" -#include "timestamp.h" -#include "user.h" #include "user-handlers.h" #include From d7909a77aa4fde9df8bb2086bc0141a03b949133 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 19 Oct 2018 09:49:23 -0700 Subject: [PATCH 02/26] GUACAMOLE-637: Add strlcpy() implementation. Use libc strlcpy() if available. --- configure.ac | 5 +++ src/libguac/Makefile.am | 2 + src/libguac/guacamole/string.h | 68 ++++++++++++++++++++++++++++++++++ src/libguac/string.c | 52 ++++++++++++++++++++++++++ 4 files changed, 127 insertions(+) create mode 100644 src/libguac/guacamole/string.h create mode 100644 src/libguac/string.c diff --git a/configure.ac b/configure.ac index b63ed690..c3ea7366 100644 --- a/configure.ac +++ b/configure.ac @@ -120,6 +120,11 @@ AC_CHECK_DECL([poll], [Whether poll() is defined])],, [#include ]) +AC_CHECK_DECL([strlcpy], + [AC_DEFINE([HAVE_STRLCPY],, + [Whether strlcpy() is defined])],, + [#include ]) + # Typedefs AC_TYPE_SIZE_T AC_TYPE_SSIZE_T diff --git a/src/libguac/Makefile.am b/src/libguac/Makefile.am index 876eb298..ffcfffc8 100644 --- a/src/libguac/Makefile.am +++ b/src/libguac/Makefile.am @@ -61,6 +61,7 @@ libguacinc_HEADERS = \ guacamole/socket-types.h \ guacamole/stream.h \ guacamole/stream-types.h \ + guacamole/string.h \ guacamole/timestamp.h \ guacamole/timestamp-types.h \ guacamole/unicode.h \ @@ -96,6 +97,7 @@ libguac_la_SOURCES = \ socket-fd.c \ socket-nest.c \ socket-tee.c \ + string.c \ timestamp.c \ unicode.c \ user.c \ diff --git a/src/libguac/guacamole/string.h b/src/libguac/guacamole/string.h new file mode 100644 index 00000000..89561ab0 --- /dev/null +++ b/src/libguac/guacamole/string.h @@ -0,0 +1,68 @@ +/* + * 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. + */ + +#ifndef GUAC_STRING_H +#define GUAC_STRING_H + +/** + * Provides convenience functions for manipulating strings. + * + * @file string.h + */ + +#include +#include + +/** + * Copies a limited number of bytes from the given source string to the given + * destination buffer. The resulting buffer will always be null-terminated, + * even if doing so means that the intended string is truncated, unless the + * destination buffer has no space available at all. As this function always + * returns the length of the string it tried to create (the length of the + * source string), whether truncation has occurred can be detected by comparing + * the return value against the size of the destination buffer. If the value + * returned is greater than or equal to the size of the destination buffer, then + * the string has been truncated. + * + * The source and destination buffers MAY NOT overlap. + * + * @param dest + * The buffer which should receive the contents of the source string. This + * buffer will always be null terminated unless zero bytes are available + * within the buffer. + * + * @param src + * The source string to copy into the destination buffer. This string MUST + * be null terminated. + * + * @param n + * The number of bytes available within the destination buffer. If this + * value is zero, no bytes will be written to the destination buffer, and + * the destination buffer may not be null terminated. In all other cases, + * the destination buffer will always be null terminated, even if doing + * so means that the copied data from the source string will be truncated. + * + * @return + * The length of the copied string (the source string) in bytes, excluding + * the null terminator. + */ +size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n); + +#endif + diff --git a/src/libguac/string.c b/src/libguac/string.c new file mode 100644 index 00000000..fbcbcf59 --- /dev/null +++ b/src/libguac/string.c @@ -0,0 +1,52 @@ +/* + * 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 "config.h" + +#include +#include + +size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n) { + +#ifdef HAVE_STRLCPY + return strlcpy(dest, src, n); +#else + /* Calculate actual length of desired string */ + size_t length = strlen(src); + + /* Copy nothing if there is no space */ + if (n <= 0) + return length; + + /* Calculate length of the string which will be copied */ + size_t copy_length = length; + if (copy_length >= n) + copy_length = n - 1; + + /* Copy only as much of string as possible, manually adding a null + * terminator */ + memcpy(dest, src, copy_length); + dest[copy_length] = '\0'; + + /* Return the overall length of the desired string */ + return length; +#endif + +} + From a78f254611ae9889928af07766b946b97e25ade5 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 19 Oct 2018 12:28:04 -0700 Subject: [PATCH 03/26] GUACAMOLE-637: Add strlcat() implementation. Use libc strlcat() if available. --- configure.ac | 5 +++++ src/libguac/guacamole/string.h | 39 ++++++++++++++++++++++++++++++++++ src/libguac/string.c | 11 ++++++++++ 3 files changed, 55 insertions(+) diff --git a/configure.ac b/configure.ac index c3ea7366..19396204 100644 --- a/configure.ac +++ b/configure.ac @@ -125,6 +125,11 @@ AC_CHECK_DECL([strlcpy], [Whether strlcpy() is defined])],, [#include ]) +AC_CHECK_DECL([strlcat], + [AC_DEFINE([HAVE_STRLCAT],, + [Whether strlcat() is defined])],, + [#include ]) + # Typedefs AC_TYPE_SIZE_T AC_TYPE_SSIZE_T diff --git a/src/libguac/guacamole/string.h b/src/libguac/guacamole/string.h index 89561ab0..b3a89c9c 100644 --- a/src/libguac/guacamole/string.h +++ b/src/libguac/guacamole/string.h @@ -64,5 +64,44 @@ */ size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n); +/** + * Appends the given source string after the end of the given destination + * string, writing at most the given number of bytes. Both the source and + * destination strings MUST be null-terminated. The resulting buffer will + * always be null-terminated, even if doing so means that the intended string + * is truncated, unless the destination buffer has no space available at all. + * As this function always returns the length of the string it tried to create + * (the length of destination and source strings added together), whether + * truncation has occurred can be detected by comparing the return value + * against the size of the destination buffer. If the value returned is greater + * than or equal to the size of the destination buffer, then the string has + * been truncated. + * + * The source and destination buffers MAY NOT overlap. + * + * @param dest + * The buffer which should be appended with the contents of the source + * string. This buffer MUST already be null-terminated and will always be + * null-terminated unless zero bytes are available within the buffer. + * + * @param src + * The source string to append to the the destination buffer. This string + * MUST be null-terminated. + * + * @param n + * The number of bytes available within the destination buffer. If this + * value is not greater than zero, no bytes will be written to the + * destination buffer, and the destination buffer may not be + * null-terminated. In all other cases, the destination buffer will always + * be null-terminated, even if doing so means that the copied data from the + * source string will be truncated. + * + * @return + * The length of the string this function tried to create (the lengths of + * the source and destination strings added together) in bytes, excluding + * the null terminator. + */ +size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n); + #endif diff --git a/src/libguac/string.c b/src/libguac/string.c index fbcbcf59..879c5267 100644 --- a/src/libguac/string.c +++ b/src/libguac/string.c @@ -50,3 +50,14 @@ size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n) { } +size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n) { + +#ifdef HAVE_STRLCPY + return strlcat(dest, src, n); +#else + int length = strnlen(dest, n); + return length + guac_strlcpy(dest + length, src, n - length); +#endif + +} + From 5bf6a1479c2481ac4e3b0b1dea847e37b2782ebd Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 19 Oct 2018 12:28:38 -0700 Subject: [PATCH 04/26] GUACAMOLE-637: Add convenience function for joining an array of strings using a given delimiter. --- src/libguac/guacamole/string.h | 48 ++++++++++++++++++++++++++++++++++ src/libguac/string.c | 24 +++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/libguac/guacamole/string.h b/src/libguac/guacamole/string.h index b3a89c9c..479a8052 100644 --- a/src/libguac/guacamole/string.h +++ b/src/libguac/guacamole/string.h @@ -103,5 +103,53 @@ size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n); */ size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n); +/** + * Concatenates each of the given strings, separated by the given delimiter, + * storing the result within a destination buffer. The number of bytes written + * will be no more than the given number of bytes, and the destination buffer + * is guaranteed to be null-terminated, even if doing so means that one or more + * of the intended strings are truncated or omitted from the end of the result, + * unless the destination buffer has no space available at all. As this + * function always returns the length of the string it tried to create (the + * length of all source strings and all delimiters added together), whether + * truncation has occurred can be detected by comparing the return value + * against the size of the destination buffer. If the value returned is greater + * than or equal to the size of the destination buffer, then the string has + * been truncated. + * + * The source strings, delimiter string, and destination buffer MAY NOT + * overlap. + * + * @param dest + * The buffer which should receive the result of joining the given strings. + * This buffer will always be null terminated unless zero bytes are + * available within the buffer. + * + * @param elements + * The elements to concatenate together, separated by the given delimiter. + * Each element MUST be null-terminated. + * + * @param nmemb + * The number of elements within the elements array. + * + * @param delim + * The delimiter to include between each pair of elements. + * + * @param n + * The number of bytes available within the destination buffer. If this + * value is not greater than zero, no bytes will be written to the + * destination buffer, and the destination buffer may not be null + * terminated. In all other cases, the destination buffer will always be + * null terminated, even if doing so means that the result will be + * truncated. + * + * @return + * The length of the string this function tried to create (the length of + * all source strings and all delimiters added together) in bytes, + * excluding the null terminator. + */ +size_t guac_strljoin(char* restrict dest, const char* restrict const* elements, + int nmemb, const char* restrict delim, size_t n); + #endif diff --git a/src/libguac/string.c b/src/libguac/string.c index 879c5267..c913b4a4 100644 --- a/src/libguac/string.c +++ b/src/libguac/string.c @@ -61,3 +61,27 @@ size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n) { } +size_t guac_strljoin(char* restrict dest, const char* restrict const* elements, + int nmemb, const char* restrict delim, size_t n) { + + int length = 0; + const char* restrict const* current = elements; + + /* If no elements are provided, nothing to do but ensure the destination + * buffer is null terminated */ + if (nmemb <= 0) + return guac_strlcpy(dest, "", n); + + /* Initialize destination buffer with first element */ + length += guac_strlcpy(dest, *current, n); + + /* Copy all remaining elements, separated by delimiter */ + for (current++; nmemb > 1; current++, nmemb--) { + length += guac_strlcat(dest + length, delim, n - length); + length += guac_strlcat(dest + length, *current, n - length); + } + + return length; + +} + From e5c1147cf6ebcc150d4a352d5245547dc2027b06 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 12 Nov 2018 15:19:33 -0800 Subject: [PATCH 05/26] GUACAMOLE-637: Replace usages of strncpy() with guac_strlcpy(). --- src/common-ssh/sftp.c | 26 +++++++++++----- src/common/clipboard.c | 4 +-- src/protocols/rdp/guac_svc/svc_service.c | 3 +- src/protocols/rdp/rdp_fs.c | 38 +++++++++++------------- src/protocols/rdp/rdp_settings.c | 9 +++--- src/protocols/rdp/rdp_stream.c | 5 ++-- src/protocols/rdp/rdp_svc.c | 14 ++++----- src/protocols/rdp/rdp_svc.h | 7 +++-- 8 files changed, 58 insertions(+), 48 deletions(-) diff --git a/src/common-ssh/sftp.c b/src/common-ssh/sftp.c index 8a53b264..51fa7cd5 100644 --- a/src/common-ssh/sftp.c +++ b/src/common-ssh/sftp.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -74,10 +75,15 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, path++; /* Copy path into component data for parsing */ - strncpy(path_component_data, path, sizeof(path_component_data) - 1); + int length = guac_strlcpy(path_component_data, path, + sizeof(path_component_data)); + + /* Fail if input path was truncated */ + if (length >= sizeof(path_component_data)) + return 1; /* Find path components within path */ - for (i = 0; i < sizeof(path_component_data) - 1; i++) { + for (i = 0; i < sizeof(path_component_data); i++) { /* If current character is a path separator, parse as component */ char c = path_component_data[i]; @@ -114,9 +120,6 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, return 1; } - /* Ensure last component is null-terminated */ - path_component_data[i] = 0; - /* Convert components back into path */ for (; path_depth > 0; path_depth--) { @@ -830,8 +833,17 @@ static int guac_common_ssh_sftp_get_handler(guac_user* user, list_state->directory = dir; list_state->filesystem = filesystem; - strncpy(list_state->directory_name, name, - sizeof(list_state->directory_name) - 1); + + int length = guac_strlcpy(list_state->directory_name, name, + sizeof(list_state->directory_name)); + + /* Bail out if directory name is too long to store */ + if (length >= sizeof(list_state->directory_name)) { + guac_user_log(user, GUAC_LOG_INFO, "Unable to read directory " + "\"%s\": Path too long", fullpath); + free(list_state); + return 0; + } /* Allocate stream for body */ guac_stream* stream = guac_user_alloc_stream(user); diff --git a/src/common/clipboard.c b/src/common/clipboard.c index eb2f5480..e02a7ca4 100644 --- a/src/common/clipboard.c +++ b/src/common/clipboard.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -131,8 +132,7 @@ void guac_common_clipboard_reset(guac_common_clipboard* clipboard, clipboard->length = 0; /* Assign given mimetype */ - strncpy(clipboard->mimetype, mimetype, sizeof(clipboard->mimetype) - 1); - clipboard->mimetype[sizeof(clipboard->mimetype) - 1] = '\0'; + guac_strlcpy(clipboard->mimetype, mimetype, sizeof(clipboard->mimetype)); pthread_mutex_unlock(&(clipboard->lock)); diff --git a/src/protocols/rdp/guac_svc/svc_service.c b/src/protocols/rdp/guac_svc/svc_service.c index 4a38cb38..c40c6c5f 100644 --- a/src/protocols/rdp/guac_svc/svc_service.c +++ b/src/protocols/rdp/guac_svc/svc_service.c @@ -29,6 +29,7 @@ #include #include #include +#include #ifdef ENABLE_WINPR #include @@ -53,7 +54,7 @@ int VirtualChannelEntry(PCHANNEL_ENTRY_POINTS pEntryPoints) { guac_rdp_svc* svc = (guac_rdp_svc*) entry_points_ex->pExtendedData; /* Init channel def */ - strncpy(svc_plugin->plugin.channel_def.name, svc->name, + guac_strlcpy(svc_plugin->plugin.channel_def.name, svc->name, GUAC_RDP_SVC_MAX_LENGTH); svc_plugin->plugin.channel_def.options = CHANNEL_OPTION_INITIALIZED diff --git a/src/protocols/rdp/rdp_fs.c b/src/protocols/rdp/rdp_fs.c index ab48cc2f..db58c77e 100644 --- a/src/protocols/rdp/rdp_fs.c +++ b/src/protocols/rdp/rdp_fs.c @@ -39,6 +39,7 @@ #include #include #include +#include #include guac_rdp_fs* guac_rdp_fs_alloc(guac_client* client, const char* drive_path, @@ -607,11 +608,11 @@ const char* guac_rdp_fs_read_dir(guac_rdp_fs* fs, int file_id) { int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { int i; - int path_depth = 0; + int path_depth = 1; char path_component_data[GUAC_RDP_FS_MAX_PATH]; - const char* path_components[64]; + const char* path_components[64] = { "" }; - const char** current_path_component = &(path_components[0]); + const char** current_path_component = &(path_components[1]); const char* current_path_component_data = &(path_component_data[0]); /* If original path is not absolute, normalization fails */ @@ -622,10 +623,15 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { path++; /* Copy path into component data for parsing */ - strncpy(path_component_data, path, sizeof(path_component_data) - 1); + int length = guac_strlcpy(path_component_data, path, + sizeof(path_component_data)); + + /* Fail if input path was truncated */ + if (length >= sizeof(path_component_data)) + return 1; /* Find path components within path */ - for (i = 0; i < sizeof(path_component_data) - 1; i++) { + for (i = 0; i < sizeof(path_component_data); i++) { /* If current character is a path separator, parse as component */ char c = path_component_data[i]; @@ -666,9 +672,6 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { return 0; } - /* Ensure last component is null-terminated */ - path_component_data[i] = 0; - /* Convert components back into path */ for (; path_depth > 0; path_depth--) { @@ -691,26 +694,19 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { int guac_rdp_fs_convert_path(const char* parent, const char* rel_path, char* abs_path) { - int i; + int length; char combined_path[GUAC_RDP_FS_MAX_PATH]; - char* current = combined_path; /* Copy parent path */ - for (i=0; i #include +#include #include #ifdef ENABLE_WINPR @@ -1242,11 +1243,11 @@ void guac_rdp_push_settings(guac_client* client, /* Client name */ if (guac_settings->client_name != NULL) { #ifdef LEGACY_RDPSETTINGS - strncpy(rdp_settings->client_hostname, guac_settings->client_name, - RDP_CLIENT_HOSTNAME_SIZE - 1); + guac_strlcpy(rdp_settings->client_hostname, guac_settings->client_name, + RDP_CLIENT_HOSTNAME_SIZE); #else - strncpy(rdp_settings->ClientHostname, guac_settings->client_name, - RDP_CLIENT_HOSTNAME_SIZE - 1); + guac_strlcpy(rdp_settings->ClientHostname, guac_settings->client_name, + RDP_CLIENT_HOSTNAME_SIZE); #endif } diff --git a/src/protocols/rdp/rdp_stream.c b/src/protocols/rdp/rdp_stream.c index 5dae3667..533ff07e 100644 --- a/src/protocols/rdp/rdp_stream.c +++ b/src/protocols/rdp/rdp_stream.c @@ -32,6 +32,7 @@ #include #include #include +#include #ifdef HAVE_FREERDP_CLIENT_CLIPRDR_H #include @@ -504,8 +505,8 @@ int guac_rdp_download_get_handler(guac_user* user, guac_object* object, rdp_stream->type = GUAC_RDP_LS_STREAM; rdp_stream->ls_status.fs = fs; rdp_stream->ls_status.file_id = file_id; - strncpy(rdp_stream->ls_status.directory_name, name, - sizeof(rdp_stream->ls_status.directory_name) - 1); + guac_strlcpy(rdp_stream->ls_status.directory_name, name, + sizeof(rdp_stream->ls_status.directory_name)); /* Allocate stream for body */ guac_stream* stream = guac_user_alloc_stream(user); diff --git a/src/protocols/rdp/rdp_svc.c b/src/protocols/rdp/rdp_svc.c index 83537a86..0a6eb246 100644 --- a/src/protocols/rdp/rdp_svc.c +++ b/src/protocols/rdp/rdp_svc.c @@ -25,6 +25,7 @@ #include #include +#include #ifdef ENABLE_WINPR #include @@ -33,7 +34,6 @@ #endif #include -#include guac_rdp_svc* guac_rdp_alloc_svc(guac_client* client, char* name) { @@ -44,16 +44,14 @@ guac_rdp_svc* guac_rdp_alloc_svc(guac_client* client, char* name) { svc->plugin = NULL; svc->output_pipe = NULL; + /* Init name */ + int name_length = guac_strlcpy(svc->name, name, GUAC_RDP_SVC_MAX_LENGTH); + /* Warn about name length */ - if (strnlen(name, GUAC_RDP_SVC_MAX_LENGTH+1) > GUAC_RDP_SVC_MAX_LENGTH) + if (name_length >= GUAC_RDP_SVC_MAX_LENGTH) guac_client_log(client, GUAC_LOG_INFO, "Static channel name \"%s\" exceeds maximum of %i characters " - "and will be truncated", - name, GUAC_RDP_SVC_MAX_LENGTH); - - /* Init name */ - strncpy(svc->name, name, GUAC_RDP_SVC_MAX_LENGTH); - svc->name[GUAC_RDP_SVC_MAX_LENGTH] = '\0'; + "and will be truncated", name, GUAC_RDP_SVC_MAX_LENGTH - 1); return svc; } diff --git a/src/protocols/rdp/rdp_svc.h b/src/protocols/rdp/rdp_svc.h index 322c4d9a..ebb3a13f 100644 --- a/src/protocols/rdp/rdp_svc.h +++ b/src/protocols/rdp/rdp_svc.h @@ -27,9 +27,10 @@ #include /** - * The maximum number of characters to allow for each channel name. + * The maximum number of bytes to allow within each channel name, including + * null terminator. */ -#define GUAC_RDP_SVC_MAX_LENGTH 7 +#define GUAC_RDP_SVC_MAX_LENGTH 8 /** * Structure describing a static virtual channel, and the corresponding @@ -50,7 +51,7 @@ typedef struct guac_rdp_svc { /** * The name of the RDP channel in use, and the name to use for each pipe. */ - char name[GUAC_RDP_SVC_MAX_LENGTH+1]; + char name[GUAC_RDP_SVC_MAX_LENGTH]; /** * The output pipe, opened when the RDP server receives a connection to From 3549da0dd10cef55833a6baaccdf06ca3521427d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 12 Nov 2018 15:38:02 -0800 Subject: [PATCH 06/26] GUACAMOLE-637: Replace usages of strncat() with guac_strlcat(). --- src/libguac/client.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libguac/client.c b/src/libguac/client.c index 71cffda8..80eb4ea7 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -30,6 +30,7 @@ #include "guacamole/protocol.h" #include "guacamole/socket.h" #include "guacamole/stream.h" +#include "guacamole/string.h" #include "guacamole/timestamp.h" #include "guacamole/user.h" #include "id.h" @@ -441,8 +442,13 @@ int guac_client_load_plugin(guac_client* client, const char* protocol) { } alias; /* Add protocol and .so suffix to protocol_lib */ - strncat(protocol_lib, protocol, GUAC_PROTOCOL_NAME_LIMIT-1); - strcat(protocol_lib, GUAC_PROTOCOL_LIBRARY_SUFFIX); + guac_strlcat(protocol_lib, protocol, sizeof(protocol_lib)); + if (guac_strlcat(protocol_lib, GUAC_PROTOCOL_LIBRARY_SUFFIX, + sizeof(protocol_lib)) >= sizeof(protocol_lib)) { + guac_error = GUAC_STATUS_NO_MEMORY; + guac_error_message = "Protocol name is too long"; + return -1; + } /* Load client plugin */ client_plugin_handle = dlopen(protocol_lib, RTLD_LAZY); From fdd3292f09f37eb12fc71f9d59470a863f9aa590 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 12 Nov 2018 16:10:24 -0800 Subject: [PATCH 07/26] GUACAMOLE-637: Simplify path translation logic. Update to use guac_strl*(). Fix return values. --- src/common-ssh/sftp.c | 138 +++++++++++++------------------------ src/protocols/rdp/rdp_fs.c | 28 +++----- src/protocols/rdp/rdp_fs.h | 5 ++ 3 files changed, 64 insertions(+), 107 deletions(-) diff --git a/src/common-ssh/sftp.c b/src/common-ssh/sftp.c index 51fa7cd5..0a24c049 100644 --- a/src/common-ssh/sftp.c +++ b/src/common-ssh/sftp.c @@ -64,12 +64,11 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, char path_component_data[GUAC_COMMON_SSH_SFTP_MAX_PATH]; const char* path_components[GUAC_COMMON_SSH_SFTP_MAX_DEPTH]; - const char** current_path_component = &(path_components[0]); - const char* current_path_component_data = &(path_component_data[0]); + const char* current_path_component_data = &(path_component_data[0]); /* If original path is not absolute, normalization fails */ if (path[0] != '\\' && path[0] != '/') - return 1; + return 0; /* Skip past leading slash */ path++; @@ -80,7 +79,7 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, /* Fail if input path was truncated */ if (length >= sizeof(path_component_data)) - return 1; + return 0; /* Find path components within path */ for (i = 0; i < sizeof(path_component_data); i++) { @@ -100,9 +99,16 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, /* Otherwise, if component not current directory, add to list */ else if (strcmp(current_path_component_data, ".") != 0 - && strcmp(current_path_component_data, "") != 0) + && strcmp(current_path_component_data, "") != 0) { + + /* Fail normalization if path is too deep */ + if (path_depth >= GUAC_COMMON_SSH_SFTP_MAX_DEPTH) + return 0; + path_components[path_depth++] = current_path_component_data; + } + /* If end of string, stop */ if (c == '\0') break; @@ -121,21 +127,9 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, } /* Convert components back into path */ - for (; path_depth > 0; path_depth--) { + guac_strljoin(fullpath, path_components, path_depth, + "/", GUAC_COMMON_SSH_SFTP_MAX_PATH); - const char* filename = *(current_path_component++); - - /* Add separator */ - *(fullpath++) = '/'; - - /* Copy string */ - while (*filename != 0) - *(fullpath++) = *(filename++); - - } - - /* Terminate absolute path */ - *(fullpath++) = 0; return 1; } @@ -232,7 +226,7 @@ static guac_protocol_status guac_sftp_get_status( static int guac_ssh_append_filename(char* fullpath, const char* path, const char* filename) { - int i; + int length; /* Disallow "." as a filename */ if (strcmp(filename, ".") == 0) @@ -242,49 +236,29 @@ static int guac_ssh_append_filename(char* fullpath, const char* path, if (strcmp(filename, "..") == 0) return 0; - /* Copy path, append trailing slash */ - for (i=0; i 0 && path[i-1] != '/') - fullpath[i++] = '/'; - break; - } - - /* Copy character if not end of string */ - fullpath[i] = c; - - } - - /* Append filename */ - for (; i 0 && fullpath[length - 1] != '/') + length += guac_strlcpy(fullpath + length, "/", + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); + + /* Append filename */ + length += guac_strlcpy(fullpath + length, filename, + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); + + /* Verify path length is within maximum */ + if (length >= GUAC_COMMON_SSH_SFTP_MAX_PATH) + return 0; /* Append was successful */ return 1; @@ -313,46 +287,30 @@ static int guac_ssh_append_filename(char* fullpath, const char* path, static int guac_ssh_append_path(char* fullpath, const char* path_a, const char* path_b) { - int i; + int length; - /* Copy path, appending a trailing slash */ - for (i = 0; i < GUAC_COMMON_SSH_SFTP_MAX_PATH; i++) { + /* Copy first half of path */ + length = guac_strlcpy(fullpath, path_a, GUAC_COMMON_SSH_SFTP_MAX_PATH); + if (length >= GUAC_COMMON_SSH_SFTP_MAX_PATH) + return 0; - char c = path_a[i]; - if (c == '\0') { - if (i > 0 && path_a[i-1] != '/') - fullpath[i++] = '/'; - break; - } - - /* Copy character if not end of string */ - fullpath[i] = c; - - } + /* Ensure path ends with trailing slash */ + if (length == 0 || fullpath[length - 1] != '/') + length += guac_strlcpy(fullpath + length, "/", + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); /* Skip past leading slashes in second path */ while (*path_b == '/') path_b++; - /* Append path */ - for (; i < GUAC_COMMON_SSH_SFTP_MAX_PATH; i++) { - - char c = *(path_b++); - if (c == '\0') - break; - - /* Append each character within path */ - fullpath[i] = c; - - } + /* Append final half of path */ + length += guac_strlcpy(fullpath + length, path_b, + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); /* Verify path length is within maximum */ - if (i == GUAC_COMMON_SSH_SFTP_MAX_PATH) + if (length >= GUAC_COMMON_SSH_SFTP_MAX_PATH) return 0; - /* Terminate path string */ - fullpath[i] = '\0'; - /* Append was successful */ return 1; diff --git a/src/protocols/rdp/rdp_fs.c b/src/protocols/rdp/rdp_fs.c index db58c77e..3319e1db 100644 --- a/src/protocols/rdp/rdp_fs.c +++ b/src/protocols/rdp/rdp_fs.c @@ -610,9 +610,8 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { int i; int path_depth = 1; char path_component_data[GUAC_RDP_FS_MAX_PATH]; - const char* path_components[64] = { "" }; + const char* path_components[GUAC_RDP_MAX_PATH_DEPTH] = { "" }; - const char** current_path_component = &(path_components[1]); const char* current_path_component_data = &(path_component_data[0]); /* If original path is not absolute, normalization fails */ @@ -648,9 +647,16 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { /* Otherwise, if component not current directory, add to list */ else if (strcmp(current_path_component_data, ".") != 0 - && strcmp(current_path_component_data, "") != 0) + && strcmp(current_path_component_data, "") != 0) { + + /* Fail normalization if path is too deep */ + if (path_depth >= GUAC_RDP_MAX_PATH_DEPTH) + return 1; + path_components[path_depth++] = current_path_component_data; + } + /* If end of string, stop */ if (c == 0) break; @@ -673,21 +679,9 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { } /* Convert components back into path */ - for (; path_depth > 0; path_depth--) { + guac_strljoin(abs_path, path_components, path_depth, + "\\", GUAC_RDP_FS_MAX_PATH); - const char* filename = *(current_path_component++); - - /* Add separator */ - *(abs_path++) = '\\'; - - /* Copy string */ - while (*filename != 0) - *(abs_path++) = *(filename++); - - } - - /* Terminate absolute path */ - *(abs_path++) = 0; return 0; } diff --git a/src/protocols/rdp/rdp_fs.h b/src/protocols/rdp/rdp_fs.h index 9168efb1..7ceb5ff9 100644 --- a/src/protocols/rdp/rdp_fs.h +++ b/src/protocols/rdp/rdp_fs.h @@ -50,6 +50,11 @@ */ #define GUAC_RDP_FS_MAX_PATH 4096 +/** + * The maximum number of directories a path may contain. + */ +#define GUAC_RDP_MAX_PATH_DEPTH 64 + /** * Error code returned when no more file IDs can be allocated. */ From dec364290590fa2e5693ead50db6f6f50805e761 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 19 Nov 2018 12:37:24 -0800 Subject: [PATCH 08/26] GUACAMOLE-637: Add unit tests for guac_strlcpy(). --- src/libguac/tests/Makefile.am | 1 + src/libguac/tests/string/strlcpy.c | 100 +++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 src/libguac/tests/string/strlcpy.c diff --git a/src/libguac/tests/Makefile.am b/src/libguac/tests/Makefile.am index 414a2f45..5d0b7024 100644 --- a/src/libguac/tests/Makefile.am +++ b/src/libguac/tests/Makefile.am @@ -42,6 +42,7 @@ test_libguac_SOURCES = \ protocol/base64_decode.c \ socket/fd_send_instruction.c \ socket/nested_send_instruction.c \ + string/strlcpy.c \ unicode/charsize.c \ unicode/read.c \ unicode/strlen.c \ diff --git a/src/libguac/tests/string/strlcpy.c b/src/libguac/tests/string/strlcpy.c new file mode 100644 index 00000000..d543ee63 --- /dev/null +++ b/src/libguac/tests/string/strlcpy.c @@ -0,0 +1,100 @@ +/* + * 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 +#include + +#include +#include + +/** + * Verify guac_strlcpy() behavior when string fits buffer without truncation. + */ +void test_string__strlcpy() { + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "Guacamole", sizeof(buffer)), 9); + CU_ASSERT_STRING_EQUAL(buffer, "Guacamole"); + CU_ASSERT_EQUAL(buffer[10], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "This is a test", sizeof(buffer)), 14); + CU_ASSERT_STRING_EQUAL(buffer, "This is a test"); + CU_ASSERT_EQUAL(buffer[15], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "X", sizeof(buffer)), 1); + CU_ASSERT_STRING_EQUAL(buffer, "X"); + CU_ASSERT_EQUAL(buffer[2], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "", sizeof(buffer)), 0); + CU_ASSERT_STRING_EQUAL(buffer, ""); + CU_ASSERT_EQUAL(buffer[1], '\xFF'); + +} + +/** + * Verify guac_strlcpy() behavior when string must be truncated to fit buffer. + */ +void test_string__strlcpy_truncate() { + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "Guacamole", 6), 9); + CU_ASSERT_STRING_EQUAL(buffer, "Guaca"); + CU_ASSERT_EQUAL(buffer[6], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "This is a test", 10), 14); + CU_ASSERT_STRING_EQUAL(buffer, "This is a"); + CU_ASSERT_EQUAL(buffer[10], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "This is ANOTHER test", 2), 20); + CU_ASSERT_STRING_EQUAL(buffer, "T"); + CU_ASSERT_EQUAL(buffer[2], '\xFF'); + +} + +/** + * Verify guac_strlcpy() behavior with zero buffer sizes. + */ +void test_string__strlcpy_nospace() { + + /* 0-byte buffer plus 1 guard byte (to test overrun) */ + char buffer[1] = { '\xFF' }; + + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "Guacamole", 0), 9); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "This is a test", 0), 14); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "X", 0), 1); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strlcpy(buffer, "", 0), 0); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + +} + From 068f33aaef01e0693bd773ead220f12e18461f16 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 19 Nov 2018 23:16:22 -0800 Subject: [PATCH 09/26] GUACAMOLE-637: Add unit tests for guac_strlcat(). --- src/libguac/tests/Makefile.am | 1 + src/libguac/tests/string/strlcat.c | 153 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 src/libguac/tests/string/strlcat.c diff --git a/src/libguac/tests/Makefile.am b/src/libguac/tests/Makefile.am index 5d0b7024..49c2415f 100644 --- a/src/libguac/tests/Makefile.am +++ b/src/libguac/tests/Makefile.am @@ -42,6 +42,7 @@ test_libguac_SOURCES = \ protocol/base64_decode.c \ socket/fd_send_instruction.c \ socket/nested_send_instruction.c \ + string/strlcat.c \ string/strlcpy.c \ unicode/charsize.c \ unicode/read.c \ diff --git a/src/libguac/tests/string/strlcat.c b/src/libguac/tests/string/strlcat.c new file mode 100644 index 00000000..739f1ee1 --- /dev/null +++ b/src/libguac/tests/string/strlcat.c @@ -0,0 +1,153 @@ +/* + * 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 +#include + +#include +#include + +/** + * Verify guac_strlcat() behavior when string fits buffer without truncation. + * The return value of each call should be the length of the resulting string. + * Each resulting string should contain the full result of the contatenation, + * including null terminator. + */ +void test_string__strlcat() { + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, "Apache "); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "Guacamole", sizeof(buffer)), 16); + CU_ASSERT_STRING_EQUAL(buffer, "Apache Guacamole"); + CU_ASSERT_EQUAL(buffer[17], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, ""); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "This is a test", sizeof(buffer)), 14); + CU_ASSERT_STRING_EQUAL(buffer, "This is a test"); + CU_ASSERT_EQUAL(buffer[15], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, "AB"); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "X", sizeof(buffer)), 3); + CU_ASSERT_STRING_EQUAL(buffer, "ABX"); + CU_ASSERT_EQUAL(buffer[4], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, "X"); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "", sizeof(buffer)), 1); + CU_ASSERT_STRING_EQUAL(buffer, "X"); + CU_ASSERT_EQUAL(buffer[2], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, ""); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "", sizeof(buffer)), 0); + CU_ASSERT_STRING_EQUAL(buffer, ""); + CU_ASSERT_EQUAL(buffer[1], '\xFF'); + +} + +/** + * Verify guac_strlcat() behavior when string must be truncated to fit buffer. + * The return value of each call should be the length that would result from + * concatenating the strings given an infinite buffer, however only as many + * characters as can fit should be appended to the string within the buffer, + * and the buffer should be null-terminated. + */ +void test_string__strlcat_truncate() { + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, "Apache "); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "Guacamole", 9), 16); + CU_ASSERT_STRING_EQUAL(buffer, "Apache G"); + CU_ASSERT_EQUAL(buffer[9], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, ""); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "This is a test", 10), 14); + CU_ASSERT_STRING_EQUAL(buffer, "This is a"); + CU_ASSERT_EQUAL(buffer[10], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + strcpy(buffer, "This "); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "is ANOTHER test", 6), 20); + CU_ASSERT_STRING_EQUAL(buffer, "This "); + CU_ASSERT_EQUAL(buffer[6], '\xFF'); + +} + +/** + * Verify guac_strlcat() behavior with zero buffer sizes. The return value of + * each call should be the size of the input string, while the buffer remains + * untouched. + */ +void test_string__strlcat_nospace() { + + /* 0-byte buffer plus 1 guard byte (to test overrun) */ + char buffer[1] = { '\xFF' }; + + CU_ASSERT_EQUAL(guac_strlcat(buffer, "Guacamole", 0), 9); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strlcat(buffer, "This is a test", 0), 14); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strlcat(buffer, "X", 0), 1); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strlcat(buffer, "", 0), 0); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + +} + +/** + * Verify guac_strlcat() behavior with unterminated buffers. With respect to + * the return value, the length of the string in the buffer should be + * considered equal to the size of the buffer, however the resulting buffer + * should not be null-terminated. + */ +void test_string__strlcat_nonull() { + + char expected[1024]; + memset(expected, 0xFF, sizeof(expected)); + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "Guacamole", 256), 265); + CU_ASSERT_NSTRING_EQUAL(buffer, expected, sizeof(expected)); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "This is a test", 37), 51); + CU_ASSERT_NSTRING_EQUAL(buffer, expected, sizeof(expected)); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "X", 12), 13); + CU_ASSERT_NSTRING_EQUAL(buffer, expected, sizeof(expected)); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strlcat(buffer, "", 100), 100); + CU_ASSERT_NSTRING_EQUAL(buffer, expected, sizeof(expected)); + +} + From 258946cd88edd5bbb226ac4af3cb6c509a2a5085 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 6 Jan 2019 15:03:54 -0800 Subject: [PATCH 10/26] GUACAMOLE-637: Correctly handle string lengths as size_t (unsigned). --- src/libguac/string.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/libguac/string.c b/src/libguac/string.c index c913b4a4..f05c4c06 100644 --- a/src/libguac/string.c +++ b/src/libguac/string.c @@ -22,6 +22,26 @@ #include #include +/** + * Returns the space remaining in a buffer assuming that the given number of + * bytes have already been written. If the number of bytes exceeds the size + * of the buffer, zero is returned. + * + * @param n + * The size of the buffer in bytes. + * + * @param length + * The number of bytes which have been written to the buffer so far. If + * the routine writing the bytes will automatically truncate its writes, + * this value may exceed the size of the buffer. + * + * @return + * The number of bytes remaining in the buffer. This value will always + * be non-negative. If the number of bytes written already exceeds the + * size of the buffer, zero will be returned. + */ +#define REMAINING(n, length) (((n) < (length)) ? 0 : ((n) - (length))) + size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n) { #ifdef HAVE_STRLCPY @@ -31,7 +51,7 @@ size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n) { size_t length = strlen(src); /* Copy nothing if there is no space */ - if (n <= 0) + if (n == 0) return length; /* Calculate length of the string which will be copied */ @@ -55,8 +75,8 @@ size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n) { #ifdef HAVE_STRLCPY return strlcat(dest, src, n); #else - int length = strnlen(dest, n); - return length + guac_strlcpy(dest + length, src, n - length); + size_t length = strnlen(dest, n); + return length + guac_strlcpy(dest + length, src, REMAINING(n, length)); #endif } @@ -64,7 +84,7 @@ size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n) { size_t guac_strljoin(char* restrict dest, const char* restrict const* elements, int nmemb, const char* restrict delim, size_t n) { - int length = 0; + size_t length = 0; const char* restrict const* current = elements; /* If no elements are provided, nothing to do but ensure the destination @@ -77,8 +97,8 @@ size_t guac_strljoin(char* restrict dest, const char* restrict const* elements, /* Copy all remaining elements, separated by delimiter */ for (current++; nmemb > 1; current++, nmemb--) { - length += guac_strlcat(dest + length, delim, n - length); - length += guac_strlcat(dest + length, *current, n - length); + length += guac_strlcat(dest + length, delim, REMAINING(n, length)); + length += guac_strlcat(dest + length, *current, REMAINING(n, length)); } return length; From e6c5da315e79581d6aacd99989470783737e4b29 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 6 Jan 2019 15:26:45 -0800 Subject: [PATCH 11/26] GUACAMOLE-637: Add unit tests for guac_strljoin(). --- src/libguac/tests/Makefile.am | 1 + src/libguac/tests/string/strljoin.c | 148 ++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 src/libguac/tests/string/strljoin.c diff --git a/src/libguac/tests/Makefile.am b/src/libguac/tests/Makefile.am index 49c2415f..01f4c4cf 100644 --- a/src/libguac/tests/Makefile.am +++ b/src/libguac/tests/Makefile.am @@ -44,6 +44,7 @@ test_libguac_SOURCES = \ socket/nested_send_instruction.c \ string/strlcat.c \ string/strlcpy.c \ + string/strljoin.c \ unicode/charsize.c \ unicode/read.c \ unicode/strlen.c \ diff --git a/src/libguac/tests/string/strljoin.c b/src/libguac/tests/string/strljoin.c new file mode 100644 index 00000000..eb7f44e7 --- /dev/null +++ b/src/libguac/tests/string/strljoin.c @@ -0,0 +1,148 @@ +/* + * 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 +#include + +#include +#include + +/** + * Array of test elements containing the strings "Apache" and "Guacamole". + */ +const char* const apache_guacamole[] = { "Apache", "Guacamole" }; + +/** + * Array of test elements containing the strings "This", "is", "a", and "test". + */ +const char* const this_is_a_test[] = { "This", "is", "a", "test" }; + +/** + * Array of four test elements containing the strings "A" and "B", each + * preceded by an empty string (""). + */ +const char* const empty_a_empty_b[] = { "", "A", "", "B" }; + +/** + * Array of test elements containing ten empty strings. + */ +const char* const empty_x10[] = { "", "", "", "", "", "", "", "", "", "" }; + +/** + * Verify guac_strljoin() behavior when string fits buffer without truncation. + * The return value of each call should be the length of the resulting string. + * Each resulting string should contain the full result of the join operation, + * including null terminator. + */ +void test_string__strljoin() { + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, apache_guacamole, 2, " ", sizeof(buffer)), 16); + CU_ASSERT_STRING_EQUAL(buffer, "Apache Guacamole"); + CU_ASSERT_EQUAL(buffer[17], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, this_is_a_test, 4, "", sizeof(buffer)), 11); + CU_ASSERT_STRING_EQUAL(buffer, "Thisisatest"); + CU_ASSERT_EQUAL(buffer[12], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, this_is_a_test, 4, "-/-", sizeof(buffer)), 20); + CU_ASSERT_STRING_EQUAL(buffer, "This-/-is-/-a-/-test"); + CU_ASSERT_EQUAL(buffer[21], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, empty_a_empty_b, 4, "/", sizeof(buffer)), 5); + CU_ASSERT_STRING_EQUAL(buffer, "/A//B"); + CU_ASSERT_EQUAL(buffer[6], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, empty_x10, 10, "/", sizeof(buffer)), 9); + CU_ASSERT_STRING_EQUAL(buffer, "/////////"); + CU_ASSERT_EQUAL(buffer[10], '\xFF'); + +} + +/** + * Verify guac_strljoin() behavior when string must be truncated to fit buffer. + * The return value of each call should be the length that would result from + * joining the strings given an infinite buffer, however only as many + * characters as can fit should be appended to the string within the buffer, + * and the buffer should be null-terminated. + */ +void test_string__strljoin_truncate() { + + char buffer[1024]; + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, apache_guacamole, 2, " ", 9), 16); + CU_ASSERT_STRING_EQUAL(buffer, "Apache G"); + CU_ASSERT_EQUAL(buffer[9], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, this_is_a_test, 4, "", 8), 11); + CU_ASSERT_STRING_EQUAL(buffer, "Thisisa"); + CU_ASSERT_EQUAL(buffer[8], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, this_is_a_test, 4, "-/-", 12), 20); + CU_ASSERT_STRING_EQUAL(buffer, "This-/-is-/"); + CU_ASSERT_EQUAL(buffer[12], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, empty_a_empty_b, 4, "/", 2), 5); + CU_ASSERT_STRING_EQUAL(buffer, "/"); + CU_ASSERT_EQUAL(buffer[2], '\xFF'); + + memset(buffer, 0xFF, sizeof(buffer)); + CU_ASSERT_EQUAL(guac_strljoin(buffer, empty_x10, 10, "/", 7), 9); + CU_ASSERT_STRING_EQUAL(buffer, "//////"); + CU_ASSERT_EQUAL(buffer[7], '\xFF'); + +} + +/** + * Verify guac_strljoin() behavior with zero buffer sizes. The return value of + * each call should be the size of the input string, while the buffer remains + * untouched. + */ +void test_string__strljoin_nospace() { + + /* 0-byte buffer plus 1 guard byte (to test overrun) */ + char buffer[1] = { '\xFF' }; + + CU_ASSERT_EQUAL(guac_strljoin(buffer, apache_guacamole, 2, " ", 0), 16); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strljoin(buffer, this_is_a_test, 4, "", 0), 11); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strljoin(buffer, this_is_a_test, 4, "-/-", 0), 20); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strljoin(buffer, empty_a_empty_b, 4, "/", 0), 5); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + + CU_ASSERT_EQUAL(guac_strljoin(buffer, empty_x10, 10, "/", 0), 9); + CU_ASSERT_EQUAL(buffer[0], '\xFF'); + +} + From 7da837b42aaf9fc67e16021d9659e133773adf41 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 18:38:20 -0800 Subject: [PATCH 12/26] GUACAMOLE-637: The __BSD_VISIBLE macro is required for strlcpy() and strlcat() to be available in libc's string.h. --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 19396204..7ea60298 100644 --- a/configure.ac +++ b/configure.ac @@ -42,6 +42,7 @@ AC_CHECK_HEADERS([fcntl.h stdlib.h string.h sys/socket.h time.h sys/time.h syslo # Source characteristics AC_DEFINE([_XOPEN_SOURCE], [700], [Uses X/Open and POSIX APIs]) +AC_DEFINE([__BSD_VISIBLE], [1], [Uses BSD-specific APIs (if available)]) # Check for whether math library is required AC_CHECK_LIB([m], [cos], From 789e3883d698d693cd1cf4165f80e8c59448d753 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 18:39:26 -0800 Subject: [PATCH 13/26] GUACAMOLE-637: Not all systems place Perl in /usr/bin. The line `#!/usr/bin/env perl` should be used for portability. --- src/protocols/rdp/keymaps/generate.pl | 2 +- util/generate-test-runner.pl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protocols/rdp/keymaps/generate.pl b/src/protocols/rdp/keymaps/generate.pl index d3764d6e..263b616a 100755 --- a/src/protocols/rdp/keymaps/generate.pl +++ b/src/protocols/rdp/keymaps/generate.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file diff --git a/util/generate-test-runner.pl b/util/generate-test-runner.pl index 7534f805..ea99f7e7 100755 --- a/util/generate-test-runner.pl +++ b/util/generate-test-runner.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/env perl # # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file From b7761e9a2e56d2b3e8261d99420c1a50da31081e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 18:41:41 -0800 Subject: [PATCH 14/26] GUACAMOLE-637: The `$^` variable is non-portable and specific to GNU Make. As otherwise POSIX-compliant platforms may not provide this variable, we shouldn't use it here. --- README-unit-testing.md | 2 +- src/common/tests/Makefile.am | 2 +- src/libguac/tests/Makefile.am | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README-unit-testing.md b/README-unit-testing.md index 5bed8721..8b4e7768 100644 --- a/README-unit-testing.md +++ b/README-unit-testing.md @@ -60,7 +60,7 @@ modified to contain a sections like the following: CLEANFILES = _generated_runner.c _generated_runner.c: $(test_myproj_SOURCES) - $(AM_V_GEN) $(GEN_RUNNER) $^ > $@ + $(AM_V_GEN) $(GEN_RUNNER) $(test_myproj_SOURCES) > $@ nodist_test_libguac_SOURCES = \ _generated_runner.c diff --git a/src/common/tests/Makefile.am b/src/common/tests/Makefile.am index 8169d7d7..a9c1b559 100644 --- a/src/common/tests/Makefile.am +++ b/src/common/tests/Makefile.am @@ -60,7 +60,7 @@ GEN_RUNNER = $(top_srcdir)/util/generate-test-runner.pl CLEANFILES = _generated_runner.c _generated_runner.c: $(test_common_SOURCES) - $(AM_V_GEN) $(GEN_RUNNER) $^ > $@ + $(AM_V_GEN) $(GEN_RUNNER) $(test_common_SOURCES) > $@ nodist_test_common_SOURCES = \ _generated_runner.c diff --git a/src/libguac/tests/Makefile.am b/src/libguac/tests/Makefile.am index 01f4c4cf..4ab3269f 100644 --- a/src/libguac/tests/Makefile.am +++ b/src/libguac/tests/Makefile.am @@ -67,7 +67,7 @@ GEN_RUNNER = $(top_srcdir)/util/generate-test-runner.pl CLEANFILES = _generated_runner.c _generated_runner.c: $(test_libguac_SOURCES) - $(AM_V_GEN) $(GEN_RUNNER) $^ > $@ + $(AM_V_GEN) $(GEN_RUNNER) $(test_libguac_SOURCES) > $@ nodist_test_libguac_SOURCES = \ _generated_runner.c From 9fb713d804c30fe9c7f404d41911f6c95a3649d0 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 18:51:53 -0800 Subject: [PATCH 15/26] GUACAMOLE-637: Correct grammar of documentation for guac_strl*() unit tests. --- src/libguac/tests/string/strlcat.c | 18 +++++++++--------- src/libguac/tests/string/strlcpy.c | 6 ++++-- src/libguac/tests/string/strljoin.c | 14 +++++++------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/libguac/tests/string/strlcat.c b/src/libguac/tests/string/strlcat.c index 739f1ee1..fb0c62aa 100644 --- a/src/libguac/tests/string/strlcat.c +++ b/src/libguac/tests/string/strlcat.c @@ -24,10 +24,10 @@ #include /** - * Verify guac_strlcat() behavior when string fits buffer without truncation. - * The return value of each call should be the length of the resulting string. - * Each resulting string should contain the full result of the contatenation, - * including null terminator. + * Verify guac_strlcat() behavior when the string fits the buffer without + * truncation. The return value of each call should be the length of the + * resulting string. Each resulting string should contain the full result of + * the contatenation, including null terminator. */ void test_string__strlcat() { @@ -66,11 +66,11 @@ void test_string__strlcat() { } /** - * Verify guac_strlcat() behavior when string must be truncated to fit buffer. - * The return value of each call should be the length that would result from - * concatenating the strings given an infinite buffer, however only as many - * characters as can fit should be appended to the string within the buffer, - * and the buffer should be null-terminated. + * Verify guac_strlcat() behavior when the string must be truncated to fit the + * buffer. The return value of each call should be the length that would result + * from concatenating the strings given an infinite buffer, however only as + * many characters as can fit should be appended to the string within the + * buffer, and the buffer should be null-terminated. */ void test_string__strlcat_truncate() { diff --git a/src/libguac/tests/string/strlcpy.c b/src/libguac/tests/string/strlcpy.c index d543ee63..1e8e01e3 100644 --- a/src/libguac/tests/string/strlcpy.c +++ b/src/libguac/tests/string/strlcpy.c @@ -24,7 +24,8 @@ #include /** - * Verify guac_strlcpy() behavior when string fits buffer without truncation. + * Verify guac_strlcpy() behavior when the string fits the buffer without + * truncation. */ void test_string__strlcpy() { @@ -53,7 +54,8 @@ void test_string__strlcpy() { } /** - * Verify guac_strlcpy() behavior when string must be truncated to fit buffer. + * Verify guac_strlcpy() behavior when the string must be truncated to fit the + * buffer. */ void test_string__strlcpy_truncate() { diff --git a/src/libguac/tests/string/strljoin.c b/src/libguac/tests/string/strljoin.c index eb7f44e7..39932885 100644 --- a/src/libguac/tests/string/strljoin.c +++ b/src/libguac/tests/string/strljoin.c @@ -45,10 +45,10 @@ const char* const empty_a_empty_b[] = { "", "A", "", "B" }; const char* const empty_x10[] = { "", "", "", "", "", "", "", "", "", "" }; /** - * Verify guac_strljoin() behavior when string fits buffer without truncation. - * The return value of each call should be the length of the resulting string. - * Each resulting string should contain the full result of the join operation, - * including null terminator. + * Verify guac_strljoin() behavior when the string fits the buffer without + * truncation. The return value of each call should be the length of the + * resulting string. Each resulting string should contain the full result of + * the join operation, including null terminator. */ void test_string__strljoin() { @@ -82,9 +82,9 @@ void test_string__strljoin() { } /** - * Verify guac_strljoin() behavior when string must be truncated to fit buffer. - * The return value of each call should be the length that would result from - * joining the strings given an infinite buffer, however only as many + * Verify guac_strljoin() behavior when the string must be truncated to fit the + * buffer. The return value of each call should be the length that would result + * from joining the strings given an infinite buffer, however only as many * characters as can fit should be appended to the string within the buffer, * and the buffer should be null-terminated. */ From ba8fd17394854cd51d458b43550d33f4dbc20802 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 18:53:06 -0800 Subject: [PATCH 16/26] GUACAMOLE-637: "concatentation" ... not "contatenation". --- src/libguac/tests/string/strlcat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libguac/tests/string/strlcat.c b/src/libguac/tests/string/strlcat.c index fb0c62aa..cf3dc330 100644 --- a/src/libguac/tests/string/strlcat.c +++ b/src/libguac/tests/string/strlcat.c @@ -27,7 +27,7 @@ * Verify guac_strlcat() behavior when the string fits the buffer without * truncation. The return value of each call should be the length of the * resulting string. Each resulting string should contain the full result of - * the contatenation, including null terminator. + * the concatenation, including null terminator. */ void test_string__strlcat() { From 350d8e5995035435a96cfae288dbfe0f41d841b7 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 19:02:19 -0800 Subject: [PATCH 17/26] GUACAMOLE-637: Document failsafe behavior of guac_strlcat() in the event the destination buffer is not terminated as required. --- src/libguac/guacamole/string.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libguac/guacamole/string.h b/src/libguac/guacamole/string.h index 479a8052..5e56e330 100644 --- a/src/libguac/guacamole/string.h +++ b/src/libguac/guacamole/string.h @@ -84,6 +84,12 @@ size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n); * string. This buffer MUST already be null-terminated and will always be * null-terminated unless zero bytes are available within the buffer. * + * As a safeguard against incorrectly-written code, in the event that the + * destination buffer is not null-terminated, this function will still stop + * before overrunning the buffer, instead behaving as if the length of the + * string in the buffer is exactly the size of the buffer. The destination + * buffer will remain untouched (and unterminated) in this case. + * * @param src * The source string to append to the the destination buffer. This string * MUST be null-terminated. From c6feef6c869c1b51d585e292296ebb3dfcfd2c9d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 23 Jan 2019 20:28:09 -0800 Subject: [PATCH 18/26] GUACAMOLE-637: Clarify purpose of initial empty path component. Fix normalization logic to ensure that empty component is always present. --- src/protocols/rdp/rdp_fs.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/protocols/rdp/rdp_fs.c b/src/protocols/rdp/rdp_fs.c index 3319e1db..6b9bf044 100644 --- a/src/protocols/rdp/rdp_fs.c +++ b/src/protocols/rdp/rdp_fs.c @@ -608,11 +608,15 @@ const char* guac_rdp_fs_read_dir(guac_rdp_fs* fs, int file_id) { int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { int i; - int path_depth = 1; - char path_component_data[GUAC_RDP_FS_MAX_PATH]; - const char* path_components[GUAC_RDP_MAX_PATH_DEPTH] = { "" }; - const char* current_path_component_data = &(path_component_data[0]); + char path_component_data[GUAC_RDP_FS_MAX_PATH]; + const char* current_path_component_data = &(path_component_data[0]); + + /* Always include a blank path component at the beginning, such that the + * eventual call to guac_strljoin() will produce an absolute path (leading + * backslash) */ + int path_depth = 1; + const char* path_components[GUAC_RDP_MAX_PATH_DEPTH] = { "" }; /* If original path is not absolute, normalization fails */ if (path[0] != '\\' && path[0] != '/') @@ -641,7 +645,7 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { /* If component refers to parent, just move up in depth */ if (strcmp(current_path_component_data, "..") == 0) { - if (path_depth > 0) + if (path_depth > 1) path_depth--; } From 24ab5ca85b4adfd26c1a489e4c943208b8bc0860 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 11:49:51 -0700 Subject: [PATCH 19/26] GUACAMOLE-637: Remove unnecessary .gitignore files. Universally exclude test output. Remove duplicated exclusions. --- .gitignore | 4 ++++ src/common/.gitignore | 4 ---- src/guacd/.gitignore | 35 ----------------------------- src/libguac/.gitignore | 4 ---- src/protocols/rdp/.gitignore | 39 --------------------------------- src/protocols/ssh/.gitignore | 36 ------------------------------ src/protocols/telnet/.gitignore | 36 ------------------------------ src/protocols/vnc/.gitignore | 36 ------------------------------ 8 files changed, 4 insertions(+), 190 deletions(-) delete mode 100644 src/protocols/rdp/.gitignore delete mode 100644 src/protocols/ssh/.gitignore delete mode 100644 src/protocols/telnet/.gitignore delete mode 100644 src/protocols/vnc/.gitignore diff --git a/.gitignore b/.gitignore index e0a6d53e..e716ee0f 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,10 @@ *.gcov *.gcno +# Test suite output +*.log +*.trs + # Backup files *~ diff --git a/src/common/.gitignore b/src/common/.gitignore index f7efbda8..b66a6a9a 100644 --- a/src/common/.gitignore +++ b/src/common/.gitignore @@ -3,7 +3,3 @@ _generated_runner.c test_common -# Test suite output -*.log -*.trs - diff --git a/src/guacd/.gitignore b/src/guacd/.gitignore index 558c3194..4f9ae75e 100644 --- a/src/guacd/.gitignore +++ b/src/guacd/.gitignore @@ -13,38 +13,3 @@ guacd.exe man/guacd.8 man/guacd.conf.5 -# Object code -*.o -*.so -*.lo -*.la - -# Backup files -*~ - -# Release files -*.tar.gz - -# Files currently being edited by vim or vi -*.swp - -# automake/autoconf -.deps/ -.libs/ -Makefile -Makefile.in -aclocal.m4 -autom4te.cache/ -m4/ -config.guess -config.log -config.status -config.sub -configure -depcomp -install-sh -libtool -ltmain.sh -missing - - diff --git a/src/libguac/.gitignore b/src/libguac/.gitignore index ab580797..9adc856b 100644 --- a/src/libguac/.gitignore +++ b/src/libguac/.gitignore @@ -3,7 +3,3 @@ _generated_runner.c test_libguac -# Test suite output -*.log -*.trs - diff --git a/src/protocols/rdp/.gitignore b/src/protocols/rdp/.gitignore deleted file mode 100644 index dd8ff144..00000000 --- a/src/protocols/rdp/.gitignore +++ /dev/null @@ -1,39 +0,0 @@ - -# Object code -*.o -*.so -*.lo -*.la - -# Backup files -*~ - -# Release files -*.tar.gz - -# Files currently being edited by vim or vi -*.swp - -# automake/autoconf -.deps/ -.libs/ -Makefile -Makefile.in -aclocal.m4 -autom4te.cache/ -m4/* -!README -config.guess -config.log -config.status -config.sub -configure -depcomp -install-sh -libtool -ltmain.sh -missing - -# Autogenerated sources -_generated_keymaps.c - diff --git a/src/protocols/ssh/.gitignore b/src/protocols/ssh/.gitignore deleted file mode 100644 index 3f725332..00000000 --- a/src/protocols/ssh/.gitignore +++ /dev/null @@ -1,36 +0,0 @@ - -# Object code -*.o -*.so -*.lo -*.la - -# Backup files -*~ - -# Release files -*.tar.gz - -# Files currently being edited by vim or vi -*.swp - -# automake/autoconf -.deps/ -.libs/ -Makefile -Makefile.in -aclocal.m4 -autom4te.cache/ -m4/* -!README -config.guess -config.log -config.status -config.sub -configure -depcomp -install-sh -libtool -ltmain.sh -missing - diff --git a/src/protocols/telnet/.gitignore b/src/protocols/telnet/.gitignore deleted file mode 100644 index 3f725332..00000000 --- a/src/protocols/telnet/.gitignore +++ /dev/null @@ -1,36 +0,0 @@ - -# Object code -*.o -*.so -*.lo -*.la - -# Backup files -*~ - -# Release files -*.tar.gz - -# Files currently being edited by vim or vi -*.swp - -# automake/autoconf -.deps/ -.libs/ -Makefile -Makefile.in -aclocal.m4 -autom4te.cache/ -m4/* -!README -config.guess -config.log -config.status -config.sub -configure -depcomp -install-sh -libtool -ltmain.sh -missing - diff --git a/src/protocols/vnc/.gitignore b/src/protocols/vnc/.gitignore deleted file mode 100644 index 3f725332..00000000 --- a/src/protocols/vnc/.gitignore +++ /dev/null @@ -1,36 +0,0 @@ - -# Object code -*.o -*.so -*.lo -*.la - -# Backup files -*~ - -# Release files -*.tar.gz - -# Files currently being edited by vim or vi -*.swp - -# automake/autoconf -.deps/ -.libs/ -Makefile -Makefile.in -aclocal.m4 -autom4te.cache/ -m4/* -!README -config.guess -config.log -config.status -config.sub -configure -depcomp -install-sh -libtool -ltmain.sh -missing - From f19754cfa6be8fde5012a77f1c8cc77500a74018 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 13:41:24 -0700 Subject: [PATCH 20/26] 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); + +} + From 591e494dfd42bdc466844ed23631932cf8a0aef9 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 13:55:15 -0700 Subject: [PATCH 21/26] GUACAMOLE-637: Add unit tests for RDP filesystem path normalization. --- configure.ac | 5 + src/protocols/rdp/.gitignore | 8 + src/protocols/rdp/Makefile.am | 1 + src/protocols/rdp/tests/Makefile.am | 64 ++++++ src/protocols/rdp/tests/fs/normalize_path.c | 216 ++++++++++++++++++++ 5 files changed, 294 insertions(+) create mode 100644 src/protocols/rdp/.gitignore create mode 100644 src/protocols/rdp/tests/Makefile.am create mode 100644 src/protocols/rdp/tests/fs/normalize_path.c diff --git a/configure.ac b/configure.ac index 13231239..505f696b 100644 --- a/configure.ac +++ b/configure.ac @@ -151,6 +151,10 @@ AC_SUBST([PULSE_INCLUDE], '-I$(top_srcdir)/src/pulse') AC_SUBST([COMMON_SSH_LTLIB], '$(top_builddir)/src/common-ssh/libguac_common_ssh.la') AC_SUBST([COMMON_SSH_INCLUDE], '-I$(top_srcdir)/src/common-ssh') +# RDP support +AC_SUBST([LIBGUAC_CLIENT_RDP_LTLIB], '$(top_builddir)/src/protocols/rdp/libguac-client-rdp.la') +AC_SUBST([LIBGUAC_CLIENT_RDP_INCLUDE], '-I$(top_srcdir)/src/protocols/rdp') + # Terminal emulator AC_SUBST([TERMINAL_LTLIB], '$(top_builddir)/src/terminal/libguac_terminal.la') AC_SUBST([TERMINAL_INCLUDE], '-I$(top_srcdir)/src/terminal $(PANGO_CFLAGS) $(PANGOCAIRO_CFLAGS) $(COMMON_INCLUDE)') @@ -1331,6 +1335,7 @@ AC_CONFIG_FILES([Makefile src/pulse/Makefile src/protocols/kubernetes/Makefile src/protocols/rdp/Makefile + src/protocols/rdp/tests/Makefile src/protocols/ssh/Makefile src/protocols/telnet/Makefile src/protocols/vnc/Makefile]) diff --git a/src/protocols/rdp/.gitignore b/src/protocols/rdp/.gitignore new file mode 100644 index 00000000..9f87ecb2 --- /dev/null +++ b/src/protocols/rdp/.gitignore @@ -0,0 +1,8 @@ + +# Auto-generated test runner and binary +_generated_runner.c +test_rdp + +# Autogenerated sources +_generated_keymaps.c + diff --git a/src/protocols/rdp/Makefile.am b/src/protocols/rdp/Makefile.am index 05a7a3ab..cbb6c4f1 100644 --- a/src/protocols/rdp/Makefile.am +++ b/src/protocols/rdp/Makefile.am @@ -27,6 +27,7 @@ AUTOMAKE_OPTIONS = foreign ACLOCAL_AMFLAGS = -I m4 lib_LTLIBRARIES = libguac-client-rdp.la +SUBDIRS = . tests nodist_libguac_client_rdp_la_SOURCES = \ _generated_keymaps.c diff --git a/src/protocols/rdp/tests/Makefile.am b/src/protocols/rdp/tests/Makefile.am new file mode 100644 index 00000000..a803b638 --- /dev/null +++ b/src/protocols/rdp/tests/Makefile.am @@ -0,0 +1,64 @@ +# +# 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 RDP support +# + +check_PROGRAMS = test_rdp +TESTS = $(check_PROGRAMS) + +test_rdp_SOURCES = \ + fs/normalize_path.c + +test_rdp_CFLAGS = \ + -Werror -Wall -pedantic \ + @LIBGUAC_CLIENT_RDP_INCLUDE@ + +test_rdp_LDADD = \ + @CUNIT_LIBS@ \ + @LIBGUAC_CLIENT_RDP_LTLIB@ + +# +# Autogenerate test runner +# + +GEN_RUNNER = $(top_srcdir)/util/generate-test-runner.pl +CLEANFILES = _generated_runner.c + +_generated_runner.c: $(test_rdp_SOURCES) + $(AM_V_GEN) $(GEN_RUNNER) $(test_rdp_SOURCES) > $@ + +nodist_test_rdp_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/protocols/rdp/tests/fs/normalize_path.c b/src/protocols/rdp/tests/fs/normalize_path.c new file mode 100644 index 00000000..0d8f17fa --- /dev/null +++ b/src/protocols/rdp/tests/fs/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 "rdp_fs.h" + +#include +#include + +/** + * Test which verifies absolute Windows-style paths are correctly normalized to + * absolute paths with Windows separators and no relative components. + */ +void test_fs__normalize_absolute_windows() { + + char normalized[GUAC_RDP_FS_MAX_PATH]; + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo\\bar\\baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\bar\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo\\bar\\..\\baz\\", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo\\bar\\..\\..\\baz\\a\\..\\b", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\baz\\b", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo\\.\\bar\\baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\bar\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo\\bar\\..\\..\\..\\..\\..\\..\\baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\baz", sizeof(normalized)); + +} + +/** + * Test which verifies absolute UNIX-style paths are correctly normalized to + * absolute paths with Windows separators and no relative components. + */ +void test_fs__normalize_absolute_unix() { + + char normalized[GUAC_RDP_FS_MAX_PATH]; + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/foo/bar/baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\bar\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/foo/bar/../baz/", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/foo/bar/../../baz/a/../b", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\baz\\b", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/foo/./bar/baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\bar\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/foo/bar/../../../../../../baz", normalized), 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 Windows + * separators and no relative components. + */ +void test_fs__normalize_absolute_mixed() { + + char normalized[GUAC_RDP_FS_MAX_PATH]; + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo/bar\\baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\bar\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("/foo\\bar/..\\baz/", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo/bar\\../../baz\\a\\..\\b", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\baz\\b", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo\\.\\bar/baz", normalized), 0) + CU_ASSERT_NSTRING_EQUAL(normalized, "\\foo\\bar\\baz", sizeof(normalized)); + + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path("\\foo/bar\\../..\\..\\..\\../..\\baz", normalized), 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_RDP_FS_MAX_PATH]; + + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(".", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("..", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("foo", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(".\\foo", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("..\\foo", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("foo\\bar\\baz", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(".\\foo\\bar\\baz", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("..\\foo\\bar\\baz", normalized), 0) + +} + +/** + * Test which verifies relative UNIX-style paths are always rejected. + */ +void test_fs__normalize_relative_unix() { + + char normalized[GUAC_RDP_FS_MAX_PATH]; + + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(".", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("..", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("foo", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("./foo", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("../foo", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("foo/bar/baz", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("./foo/bar/baz", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("../foo/bar/baz", normalized), 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_RDP_FS_MAX_PATH]; + + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("foo\\bar/baz", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(".\\foo/bar/baz", normalized), 0) + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path("../foo\\bar\\baz", normalized), 0) + +} + +/** + * Generates a dynamically-allocated path having the given number of bytes, not + * counting the null-terminator. The path will contain only Windows-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_RDP_FS_MAX_PATH]; + + /* Exceeds maximum length by a factor of 2 */ + input = generate_path(GUAC_RDP_FS_MAX_PATH*2); + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); + free(input); + + /* Exceeds maximum length by one byte */ + input = generate_path(GUAC_RDP_FS_MAX_PATH); + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); + free(input); + + /* Exactly maximum length */ + input = generate_path(GUAC_RDP_FS_MAX_PATH - 1); + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); + free(input); + +} + From 159198057993c1f06af3879923bb3de7895a97b0 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 16:14:00 -0700 Subject: [PATCH 22/26] GUACAMOLE-637: Simplify SFTP path normalization logic. Correct behavior to match documentation. --- src/common-ssh/sftp.c | 57 +++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/src/common-ssh/sftp.c b/src/common-ssh/sftp.c index 786ccc8d..b05409b5 100644 --- a/src/common-ssh/sftp.c +++ b/src/common-ssh/sftp.c @@ -36,77 +36,66 @@ int guac_common_ssh_sftp_normalize_path(char* fullpath, const char* path) { - int i; - int path_depth = 0; - char path_component_data[GUAC_COMMON_SSH_SFTP_MAX_PATH]; const char* path_components[GUAC_COMMON_SSH_SFTP_MAX_DEPTH]; - const char* current_path_component_data = &(path_component_data[0]); - /* If original path is not absolute, normalization fails */ if (path[0] != '\\' && path[0] != '/') return 0; - /* Skip past leading slash */ - path++; + /* Create scratch copy of path excluding leading slash (we will be + * replacing path separators with null terminators and referencing those + * substrings directly as path components) */ + char path_scratch[GUAC_COMMON_SSH_SFTP_MAX_PATH - 1]; + int length = guac_strlcpy(path_scratch, path + 1, + sizeof(path_scratch)); - /* Copy path into component data for parsing */ - int length = guac_strlcpy(path_component_data, path, - sizeof(path_component_data)); - - /* Fail if input path was truncated */ - if (length >= sizeof(path_component_data)) + /* Fail if provided path is too long */ + if (length >= sizeof(path_scratch)) return 0; - /* Find path components within path */ - for (i = 0; i < sizeof(path_component_data); i++) { + /* Locate all path components within path */ + const char* current_path_component = &(path_scratch[0]); + for (int i = 0; i <= length; i++) { /* If current character is a path separator, parse as component */ - char c = path_component_data[i]; + char c = path_scratch[i]; if (c == '/' || c == '\\' || c == '\0') { /* Terminate current component */ - path_component_data[i] = '\0'; + path_scratch[i] = '\0'; /* If component refers to parent, just move up in depth */ - if (strcmp(current_path_component_data, "..") == 0) { + if (strcmp(current_path_component, "..") == 0) { if (path_depth > 0) path_depth--; } /* Otherwise, if component not current directory, add to list */ - else if (strcmp(current_path_component_data, ".") != 0 - && strcmp(current_path_component_data, "") != 0) { + else if (strcmp(current_path_component, ".") != 0 + && strcmp(current_path_component, "") != 0) { /* Fail normalization if path is too deep */ if (path_depth >= GUAC_COMMON_SSH_SFTP_MAX_DEPTH) return 0; - path_components[path_depth++] = current_path_component_data; + path_components[path_depth++] = current_path_component; } - /* If end of string, stop */ - if (c == '\0') - break; - /* Update start of next component */ - current_path_component_data = &(path_component_data[i+1]); + current_path_component = &(path_scratch[i+1]); } /* end if separator */ } /* end for each character */ - /* If no components, the path is simply root */ - if (path_depth == 0) { - strcpy(fullpath, "/"); - return 1; - } + /* Add leading slash for resulting absolute path */ + fullpath[0] = '/'; - /* Convert components back into path */ - guac_strljoin(fullpath, path_components, path_depth, - "/", GUAC_COMMON_SSH_SFTP_MAX_PATH); + /* Append normalized components to path, separated by slashes */ + guac_strljoin(fullpath + 1, path_components, path_depth, + "/", GUAC_COMMON_SSH_SFTP_MAX_PATH - 1); return 1; From 986f7f5d6468f1344507c7b82a1df10cd44d2c27 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 16:30:27 -0700 Subject: [PATCH 23/26] GUACAMOLE-637: Use same logic for RDP filesystem path normalization as SFTP. --- src/protocols/rdp/rdp_fs.c | 68 +++++++++++++++----------------------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/src/protocols/rdp/rdp_fs.c b/src/protocols/rdp/rdp_fs.c index 6b9bf044..0e7345f9 100644 --- a/src/protocols/rdp/rdp_fs.c +++ b/src/protocols/rdp/rdp_fs.c @@ -607,66 +607,55 @@ const char* guac_rdp_fs_read_dir(guac_rdp_fs* fs, int file_id) { int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { - int i; - - char path_component_data[GUAC_RDP_FS_MAX_PATH]; - const char* current_path_component_data = &(path_component_data[0]); - - /* Always include a blank path component at the beginning, such that the - * eventual call to guac_strljoin() will produce an absolute path (leading - * backslash) */ - int path_depth = 1; - const char* path_components[GUAC_RDP_MAX_PATH_DEPTH] = { "" }; + int path_depth = 0; + const char* path_components[GUAC_RDP_MAX_PATH_DEPTH]; /* If original path is not absolute, normalization fails */ if (path[0] != '\\' && path[0] != '/') return 1; - /* Skip past leading slash */ - path++; + /* Create scratch copy of path excluding leading slash (we will be + * replacing path separators with null terminators and referencing those + * substrings directly as path components) */ + char path_scratch[GUAC_RDP_FS_MAX_PATH - 1]; + int length = guac_strlcpy(path_scratch, path + 1, + sizeof(path_scratch)); - /* Copy path into component data for parsing */ - int length = guac_strlcpy(path_component_data, path, - sizeof(path_component_data)); - - /* Fail if input path was truncated */ - if (length >= sizeof(path_component_data)) + /* Fail if provided path is too long */ + if (length >= sizeof(path_scratch)) return 1; - /* Find path components within path */ - for (i = 0; i < sizeof(path_component_data); i++) { + /* Locate all path components within path */ + const char* current_path_component = &(path_scratch[0]); + for (int i = 0; i <= length; i++) { /* If current character is a path separator, parse as component */ - char c = path_component_data[i]; - if (c == '/' || c == '\\' || c == 0) { + char c = path_scratch[i]; + if (c == '/' || c == '\\' || c == '\0') { /* Terminate current component */ - path_component_data[i] = 0; + path_scratch[i] = '\0'; /* If component refers to parent, just move up in depth */ - if (strcmp(current_path_component_data, "..") == 0) { - if (path_depth > 1) + if (strcmp(current_path_component, "..") == 0) { + if (path_depth > 0) path_depth--; } /* Otherwise, if component not current directory, add to list */ - else if (strcmp(current_path_component_data, ".") != 0 - && strcmp(current_path_component_data, "") != 0) { + else if (strcmp(current_path_component, ".") != 0 + && strcmp(current_path_component, "") != 0) { /* Fail normalization if path is too deep */ if (path_depth >= GUAC_RDP_MAX_PATH_DEPTH) return 1; - path_components[path_depth++] = current_path_component_data; + path_components[path_depth++] = current_path_component; } - /* If end of string, stop */ - if (c == 0) - break; - /* Update start of next component */ - current_path_component_data = &(path_component_data[i+1]); + current_path_component = &(path_scratch[i+1]); } /* end if separator */ @@ -676,15 +665,12 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { } /* end for each character */ - /* If no components, the path is simply root */ - if (path_depth == 0) { - strcpy(abs_path, "\\"); - return 0; - } + /* Add leading slash for resulting absolute path */ + abs_path[0] = '\\'; - /* Convert components back into path */ - guac_strljoin(abs_path, path_components, path_depth, - "\\", GUAC_RDP_FS_MAX_PATH); + /* Append normalized components to path, separated by slashes */ + guac_strljoin(abs_path + 1, path_components, path_depth, + "\\", GUAC_RDP_FS_MAX_PATH - 1); return 0; From 6e2be38ae2bc79623dc7412a3547788d1ea1f31f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 16:36:16 -0700 Subject: [PATCH 24/26] GUACAMOLE-637: Add path depth limits to generated paths in unit tests. --- src/common-ssh/tests/sftp/normalize_path.c | 21 +++++++++++++++------ src/protocols/rdp/tests/fs/normalize_path.c | 21 +++++++++++++++------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/common-ssh/tests/sftp/normalize_path.c b/src/common-ssh/tests/sftp/normalize_path.c index 23975252..79f4d426 100644 --- a/src/common-ssh/tests/sftp/normalize_path.c +++ b/src/common-ssh/tests/sftp/normalize_path.c @@ -166,19 +166,28 @@ void test_fs__normalize_relative_mixed() { * The number of bytes to include in the generated path, not counting the * null-terminator. * + * @param max_depth + * The maximum number of path components to include within the generated + * path. + * * @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) { +static char* generate_path(int length, int max_depth) { int i; char* input = malloc(length + 1); - /* Fill path with /x/x/x/x/x/x/x/x/x/x/... */ + /* Fill path with /x/x/x/x/x/x/x/x/x/x/.../xxxxxxxxx... */ for (i = 0; i < length; i++) { - input[i] = (i % 2 == 0) ? '/' : 'x'; + if (max_depth > 0 && i % 2 == 0) { + input[i] = '/'; + max_depth--; + } + else + input[i] = 'x'; } /* Add null terminator */ @@ -198,17 +207,17 @@ void test_fs__normalize_long() { 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); + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH*2, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); 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); + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); 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); + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH - 1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); free(input); diff --git a/src/protocols/rdp/tests/fs/normalize_path.c b/src/protocols/rdp/tests/fs/normalize_path.c index 0d8f17fa..02fbc6a6 100644 --- a/src/protocols/rdp/tests/fs/normalize_path.c +++ b/src/protocols/rdp/tests/fs/normalize_path.c @@ -166,19 +166,28 @@ void test_fs__normalize_relative_mixed() { * The number of bytes to include in the generated path, not counting the * null-terminator. * + * @param max_depth + * The maximum number of path components to include within the generated + * path. + * * @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) { +static char* generate_path(int length, int max_depth) { int i; char* input = malloc(length + 1); - /* Fill path with \x\x\x\x\x\x\x\x\x\x\... */ + /* Fill path with \x\x\x\x\x\x\x\x\x\x\...\xxxxxxxxx... */ for (i = 0; i < length; i++) { - input[i] = (i % 2 == 0) ? '\\' : 'x'; + if (max_depth > 0 && i % 2 == 0) { + input[i] = '\\'; + max_depth--; + } + else + input[i] = 'x'; } /* Add null terminator */ @@ -198,17 +207,17 @@ void test_fs__normalize_long() { char normalized[GUAC_RDP_FS_MAX_PATH]; /* Exceeds maximum length by a factor of 2 */ - input = generate_path(GUAC_RDP_FS_MAX_PATH*2); + input = generate_path(GUAC_RDP_FS_MAX_PATH*2, GUAC_RDP_MAX_PATH_DEPTH); CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); free(input); /* Exceeds maximum length by one byte */ - input = generate_path(GUAC_RDP_FS_MAX_PATH); + input = generate_path(GUAC_RDP_FS_MAX_PATH, GUAC_RDP_MAX_PATH_DEPTH); CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); free(input); /* Exactly maximum length */ - input = generate_path(GUAC_RDP_FS_MAX_PATH - 1); + input = generate_path(GUAC_RDP_FS_MAX_PATH - 1, GUAC_RDP_MAX_PATH_DEPTH); CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); free(input); From cda7bca126f8d5e9b7b200d34fe8a648fe752d51 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 16:51:33 -0700 Subject: [PATCH 25/26] GUACAMOLE-637: Add RDP filesystem and SFTP unit tests for path depth. --- src/common-ssh/tests/sftp/normalize_path.c | 42 ++++++++++++++++++++- src/protocols/rdp/tests/fs/normalize_path.c | 35 ++++++++++++++++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/common-ssh/tests/sftp/normalize_path.c b/src/common-ssh/tests/sftp/normalize_path.c index 79f4d426..a33dbad6 100644 --- a/src/common-ssh/tests/sftp/normalize_path.c +++ b/src/common-ssh/tests/sftp/normalize_path.c @@ -164,7 +164,8 @@ void test_fs__normalize_relative_mixed() { * * @param length * The number of bytes to include in the generated path, not counting the - * null-terminator. + * null-terminator. If -1, the length of the path will be automatically + * determined from the provided max_depth. * * @param max_depth * The maximum number of path components to include within the generated @@ -177,6 +178,10 @@ void test_fs__normalize_relative_mixed() { */ static char* generate_path(int length, int max_depth) { + /* If no length given, calculate space required from max_depth */ + if (length == -1) + length = max_depth * 2; + int i; char* input = malloc(length + 1); @@ -207,7 +212,7 @@ void test_fs__normalize_long() { 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, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); + input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH * 2, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); free(input); @@ -223,3 +228,36 @@ void test_fs__normalize_long() { } +/** + * Test which verifies that paths exceeding the maximum path depth are + * rejected. + */ +void test_fs__normalize_deep() { + + char* input; + char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; + + /* Exceeds maximum depth by a factor of 2 */ + input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH * 2); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + + /* Exceeds maximum depth by one component */ + input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH + 1); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + + /* Exactly maximum depth (should still be rejected as SFTP depth limits are + * set such that a path with the maximum depth will exceed the maximum + * length) */ + input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); + CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + + /* Less than maximum depth */ + input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH - 1); + CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); + free(input); + +} + diff --git a/src/protocols/rdp/tests/fs/normalize_path.c b/src/protocols/rdp/tests/fs/normalize_path.c index 02fbc6a6..ccf23e01 100644 --- a/src/protocols/rdp/tests/fs/normalize_path.c +++ b/src/protocols/rdp/tests/fs/normalize_path.c @@ -164,7 +164,8 @@ void test_fs__normalize_relative_mixed() { * * @param length * The number of bytes to include in the generated path, not counting the - * null-terminator. + * null-terminator. If -1, the length of the path will be automatically + * determined from the provided max_depth. * * @param max_depth * The maximum number of path components to include within the generated @@ -177,6 +178,10 @@ void test_fs__normalize_relative_mixed() { */ static char* generate_path(int length, int max_depth) { + /* If no length given, calculate space required from max_depth */ + if (length == -1) + length = max_depth * 2; + int i; char* input = malloc(length + 1); @@ -207,7 +212,7 @@ void test_fs__normalize_long() { char normalized[GUAC_RDP_FS_MAX_PATH]; /* Exceeds maximum length by a factor of 2 */ - input = generate_path(GUAC_RDP_FS_MAX_PATH*2, GUAC_RDP_MAX_PATH_DEPTH); + input = generate_path(GUAC_RDP_FS_MAX_PATH * 2, GUAC_RDP_MAX_PATH_DEPTH); CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); free(input); @@ -223,3 +228,29 @@ void test_fs__normalize_long() { } +/** + * Test which verifies that paths exceeding the maximum path depth are + * rejected. + */ +void test_fs__normalize_deep() { + + char* input; + char normalized[GUAC_RDP_FS_MAX_PATH]; + + /* Exceeds maximum depth by a factor of 2 */ + input = generate_path(-1, GUAC_RDP_MAX_PATH_DEPTH * 2); + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); + free(input); + + /* Exceeds maximum depth by one component */ + input = generate_path(-1, GUAC_RDP_MAX_PATH_DEPTH + 1); + CU_ASSERT_NOT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); + free(input); + + /* Exactly maximum depth */ + input = generate_path(-1, GUAC_RDP_MAX_PATH_DEPTH); + CU_ASSERT_EQUAL(guac_rdp_fs_normalize_path(input, normalized), 0); + free(input); + +} + From f8ec709e334847c96cdc649a2ee43e5a42a3deb2 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 7 Apr 2019 16:51:53 -0700 Subject: [PATCH 26/26] GUACAMOLE-637: Correct naming of SFTP unit tests. --- src/common-ssh/tests/sftp/normalize_path.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/common-ssh/tests/sftp/normalize_path.c b/src/common-ssh/tests/sftp/normalize_path.c index a33dbad6..151b1835 100644 --- a/src/common-ssh/tests/sftp/normalize_path.c +++ b/src/common-ssh/tests/sftp/normalize_path.c @@ -26,7 +26,7 @@ * 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() { +void test_sftp__normalize_absolute_windows() { char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -54,7 +54,7 @@ void test_fs__normalize_absolute_windows() { * 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() { +void test_sftp__normalize_absolute_unix() { char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -83,7 +83,7 @@ void test_fs__normalize_absolute_unix() { * separators are correctly normalized to absolute paths with UNIX separators * and no relative components. */ -void test_fs__normalize_absolute_mixed() { +void test_sftp__normalize_absolute_mixed() { char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -107,7 +107,7 @@ void test_fs__normalize_absolute_mixed() { /** * Test which verifies relative Windows-style paths are always rejected. */ -void test_fs__normalize_relative_windows() { +void test_sftp__normalize_relative_windows() { char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -126,7 +126,7 @@ void test_fs__normalize_relative_windows() { /** * Test which verifies relative UNIX-style paths are always rejected. */ -void test_fs__normalize_relative_unix() { +void test_sftp__normalize_relative_unix() { char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -146,7 +146,7 @@ void test_fs__normalize_relative_unix() { * Test which verifies relative paths consisting of mixed Windows and UNIX path * separators are always rejected. */ -void test_fs__normalize_relative_mixed() { +void test_sftp__normalize_relative_mixed() { char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -206,7 +206,7 @@ static char* generate_path(int length, int max_depth) { * Test which verifies that paths exceeding the maximum path length are * rejected. */ -void test_fs__normalize_long() { +void test_sftp__normalize_long() { char* input; char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH]; @@ -232,7 +232,7 @@ void test_fs__normalize_long() { * Test which verifies that paths exceeding the maximum path depth are * rejected. */ -void test_fs__normalize_deep() { +void test_sftp__normalize_deep() { char* input; char normalized[GUAC_COMMON_SSH_SFTP_MAX_PATH];