Sanitize suspicious path components instead of rejecting them

Apply the same rules on all the supported platforms to avoid issues
with network shares and alien file systems.

For future compatibility, explicitly mark adjusted paths as renamed.

Fixes: #294

libtransmission/metainfo-test.c

@@ -9,6 +9,7 @@
 #include "libtransmission-test.h"
 
 #include "transmission.h"
+#include "metainfo.h"
 #include "utils.h"
 
 #include <errno.h>
@@ -66,16 +67,16 @@ static int test_metainfo(void)
         { 0, TR_PARSE_OK, BEFORE_PATH "0:5:a.txt" AFTER_PATH },
         { 0, TR_PARSE_ERR, BEFORE_PATH "0:0:" AFTER_PATH },
 
-        /* don't allow path components in a filename */
+        /* allow path separators in a filename (replaced with '_') */
-        { 0, TR_PARSE_ERR, BEFORE_PATH "7:a/a.txt" AFTER_PATH },
+        { 0, TR_PARSE_OK, BEFORE_PATH "7:a/a.txt" AFTER_PATH },
 
-        /* fail on "." components */
+        /* allow "." components (skipped) */
-        { 0, TR_PARSE_ERR, BEFORE_PATH "1:.5:a.txt" AFTER_PATH },
+        { 0, TR_PARSE_OK, BEFORE_PATH "1:.5:a.txt" AFTER_PATH },
-        { 0, TR_PARSE_ERR, BEFORE_PATH "5:a.txt1:." AFTER_PATH },
+        { 0, TR_PARSE_OK, BEFORE_PATH "5:a.txt1:." AFTER_PATH },
 
-        /* fail on ".." components */
+        /* allow ".." components (replaced with "__") */
-        { 0, TR_PARSE_ERR, BEFORE_PATH "2:..5:a.txt" AFTER_PATH },
+        { 0, TR_PARSE_OK, BEFORE_PATH "2:..5:a.txt" AFTER_PATH },
-        { 0, TR_PARSE_ERR, BEFORE_PATH "5:a.txt2:.." AFTER_PATH },
+        { 0, TR_PARSE_OK, BEFORE_PATH "5:a.txt2:.." AFTER_PATH },
 
         /* fail on empty string */
         { EILSEQ, TR_PARSE_ERR, "" }
@@ -101,12 +102,76 @@ static int test_metainfo(void)
     return 0;
 }
 
+static int test_sanitize(void)
+{
+    struct
+    {
+        char const* str;
+        size_t len;
+        char const* expected_result;
+        bool expected_is_adjusted;
+    }
+    const test_data[] =
+    {
+        /* skipped */
+        { "", 0, NULL, false },
+        { ".", 1, NULL, false },
+        { "..", 2, NULL, true },
+        { ".....", 5, NULL, false },
+        { "  ", 2, NULL, false },
+        { " . ", 3, NULL, false },
+        { ". . .", 5, NULL, false },
+        /* replaced with '_'  */
+        { "/", 1, "_", true },
+        { "////", 4, "____", true },
+        { "\\\\", 2, "__", true },
+        { "/../", 4, "_.._", true },
+        { "foo<bar:baz/boo", 15, "foo_bar_baz_boo", true },
+        { "t\0e\x01s\tt\ri\nn\fg", 13, "t_e_s_t_i_n_g", true },
+        /* appended with '_' */
+        { "con", 3, "con_", true },
+        { "cOm4", 4, "cOm4_", true },
+        { "LPt9.txt", 8, "LPt9_.txt", true },
+        { "NUL.tar.gz", 10, "NUL_.tar.gz", true },
+        /* trimmed */
+        { " foo", 4, "foo", true },
+        { "foo ", 4, "foo", true },
+        { " foo ", 5, "foo", true },
+        { "foo.", 4, "foo", true },
+        { "foo...", 6, "foo", true },
+        { " foo... ", 8, "foo", true },
+        /* unmodified */
+        { "foo", 3, "foo", false },
+        { ".foo", 4, ".foo", false },
+        { "..foo", 5, "..foo", false },
+        { "foo.bar.baz", 11, "foo.bar.baz", false }
+    };
+
+    for (size_t i = 0; i < TR_N_ELEMENTS(test_data); ++i)
+    {
+        bool is_adjusted;
+        char* const result = tr_metainfo_sanitize_path_component(test_data[i].str, test_data[i].len, &is_adjusted);
+
+        check_str(result, ==, test_data[i].expected_result);
+
+        if (test_data[i].expected_result != NULL)
+        {
+            check_bool(is_adjusted, ==, test_data[i].expected_is_adjusted);
+        }
+
+        tr_free(result);
+    }
+
+    return 0;
+}
+
 int main(void)
 {
     testFunc const tests[] =
     {
         test_magnet_link,
-        test_metainfo
+        test_metainfo,
+        test_sanitize
     };
 
     return runTests(tests, NUM_TESTS(tests));

libtransmission/metainfo.c

@@ -86,21 +86,90 @@ static char* getTorrentFilename(tr_session const* session, tr_info const* inf, e
 ****
 ***/
 
-static bool path_component_is_suspicious(char const* component)
+char* tr_metainfo_sanitize_path_component(char const* str, size_t len, bool* is_adjusted)
 {
-    return component == NULL || strpbrk(component, PATH_DELIMITER_CHARS) != NULL || strcmp(component, ".") == 0 ||
+    if (len == 0 || (len == 1 && str[0] == '.'))
-        strcmp(component, "..") == 0;
+    {
+        return NULL;
+    }
+
+    *is_adjusted = false;
+
+    /* https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file */
+    char const* const reserved_chars = "<>:\"/\\|?*";
+    char const* const reserved_names[] =
+    {
+        "CON", "PRN", "AUX", "NUL",
+        "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
+        "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"
+    };
+
+    char* const ret = tr_new(char, len + 2);
+    memcpy(ret, str, len);
+    ret[len] = '\0';
+
+    for (size_t i = 0; i < len; ++i)
+    {
+        if (strchr(reserved_chars, ret[i]) != NULL || (unsigned char)ret[i] < 0x20)
+        {
+            ret[i] = '_';
+            *is_adjusted = true;
+        }
+    }
+
+    for (size_t i = 0; i < TR_N_ELEMENTS(reserved_names); ++i)
+    {
+        size_t const reserved_name_len = strlen(reserved_names[i]);
+        if (evutil_ascii_strncasecmp(ret, reserved_names[i], reserved_name_len) != 0 ||
+            (ret[reserved_name_len] != '\0' && ret[reserved_name_len] != '.'))
+        {
+            continue;
+        }
+
+        memmove(&ret[reserved_name_len + 1], &ret[reserved_name_len], len - reserved_name_len + 1);
+        ret[reserved_name_len] = '_';
+        *is_adjusted = true;
+        ++len;
+        break;
+    }
+
+    size_t start_pos = 0;
+    size_t end_pos = len;
+
+    while (start_pos < len && ret[start_pos] == ' ')
+    {
+        ++start_pos;
+    }
+
+    while (end_pos > start_pos && (ret[end_pos - 1] == ' ' || ret[end_pos - 1] == '.'))
+    {
+        --end_pos;
+    }
+
+    if (start_pos == end_pos)
+    {
+        tr_free(ret);
+        return NULL;
+    }
+
+    if (start_pos != 0 || end_pos != len)
+    {
+        len = end_pos - start_pos;
+        memmove(ret, &ret[start_pos], len);
+        ret[len] = '\0';
+        *is_adjusted = true;
+    }
+
+    return ret;
 }
 
-static bool getfile(char** setme, char const* root, tr_variant* path, struct evbuffer* buf)
+static bool getfile(char** setme, bool* is_adjusted, char const* root, tr_variant* path, struct evbuffer* buf)
 {
-    /* root's already been checked by caller */
-    TR_ASSERT(!path_component_is_suspicious(root));
-
     bool success = false;
     size_t root_len = 0;
 
     *setme = NULL;
+    *is_adjusted = false;
 
     if (tr_variantIsList(path))
     {
@@ -114,19 +183,25 @@ static bool getfile(char** setme, char const* root, tr_variant* path, struct evb
             size_t len;
             char const* str;
 
-            if (!tr_variantGetStr(tr_variantListChild(path, i), &str, &len) || path_component_is_suspicious(str))
+            if (!tr_variantGetStr(tr_variantListChild(path, i), &str, &len))
             {
                 success = false;
                 break;
             }
 
-            if (*str == '\0')
+            bool is_component_adjusted;
+            char* final_str = tr_metainfo_sanitize_path_component(str, len, &is_component_adjusted);
+            if (final_str == NULL)
             {
                 continue;
             }
 
+            *is_adjusted = *is_adjusted || is_component_adjusted;
+
             evbuffer_add(buf, TR_PATH_DELIMITER_STR, 1);
-            evbuffer_add(buf, str, len);
+            evbuffer_add(buf, final_str, strlen(final_str));
+
+            tr_free(final_str);
         }
     }
 
@@ -137,8 +212,15 @@ static bool getfile(char** setme, char const* root, tr_variant* path, struct evb
 
     if (success)
     {
-        *setme = tr_utf8clean((char*)evbuffer_pullup(buf, -1), evbuffer_get_length(buf));
+        char const* const buf_data = (char*)evbuffer_pullup(buf, -1);
-        /* fprintf(stderr, "[%s]\n", *setme); */
+        size_t const buf_len = evbuffer_get_length(buf);
+
+        *setme = tr_utf8clean(buf_data, buf_len);
+
+        if (!*is_adjusted)
+        {
+            *is_adjusted = buf_len != strlen(*setme) || strncmp(buf_data, *setme, buf_len) != 0;
+        }
     }
 
     return success;
@@ -150,16 +232,19 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
 
     inf->totalSize = 0;
 
-    if (tr_variantIsList(files)) /* multi-file mode */
+    bool is_root_adjusted;
-    {
+    char* const root_name = tr_metainfo_sanitize_path_component(inf->name, strlen(inf->name), &is_root_adjusted);
-        struct evbuffer* buf;
+    if (root_name == NULL)
-        char const* result;
-
-        if (path_component_is_suspicious(inf->name))
     {
         return "path";
     }
 
+    char const* result = NULL;
+
+    if (tr_variantIsList(files)) /* multi-file mode */
+    {
+        struct evbuffer* buf;
+
         buf = evbuffer_new();
         result = NULL;
 
@@ -189,7 +274,8 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
                 }
             }
 
-            if (!getfile(&inf->files[i].name, inf->name, path, buf))
+            bool is_file_adjusted;
+            if (!getfile(&inf->files[i].name, &is_file_adjusted, root_name, path, buf))
             {
                 result = "path";
                 break;
@@ -202,32 +288,29 @@ static char const* parseFiles(tr_info* inf, tr_variant* files, tr_variant const*
             }
 
             inf->files[i].length = len;
+            inf->files[i].is_renamed = is_root_adjusted || is_file_adjusted;
             inf->totalSize += len;
         }
 
         evbuffer_free(buf);
-        return result;
     }
     else if (tr_variantGetInt(length, &len)) /* single-file mode */
     {
-        if (path_component_is_suspicious(inf->name))
-        {
-            return "path";
-        }
-
         inf->isFolder = false;
         inf->fileCount = 1;
         inf->files = tr_new0(tr_file, 1);
-        inf->files[0].name = tr_strdup(inf->name);
+        inf->files[0].name = tr_strdup(root_name);
         inf->files[0].length = len;
+        inf->files[0].is_renamed = is_root_adjusted;
         inf->totalSize += len;
     }
     else
     {
-        return "length";
+        result = "length";
     }
 
-    return NULL;
+    tr_free(root_name);
+    return result;
 }
 
 static char* tr_convertAnnounceToScrape(char const* announce)

libtransmission/metainfo.h

@@ -30,3 +30,6 @@ char* tr_metainfoGetBasename(tr_info const*, enum tr_metainfo_basename_format fo
 
 void tr_metainfoMigrateFile(tr_session const* session, tr_info const* info, enum tr_metainfo_basename_format old_format,
     enum tr_metainfo_basename_format new_format);
+
+/** @brief Private function that's exposed here only for unit tests */
+char* tr_metainfo_sanitize_path_component(char const* str, size_t len, bool* is_adjusted);