[vlc-devel] [PATCH] url: fix vlc_UrlParse for smb/nfs/ftp/sftp

[Rémi Denis-Courmont]

Le 4 décembre 2017 16:50:11 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>Don't only authorize special characters allowed by the http protocol.
>Fixes #18991
>---
> src/text/url.c | 44 ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 38 insertions(+), 6 deletions(-)
>
>diff --git a/src/text/url.c b/src/text/url.c
>index dc983c8861..93f4b10736 100644
>--- a/src/text/url.c
>+++ b/src/text/url.c
>@@ -396,9 +396,25 @@ static bool vlc_uri_host_validate(const char *str)
>     return vlc_uri_component_validate(str, ":");
> }
> 
>-static bool vlc_uri_path_validate(const char *str)
>+static const struct scheme_pathextras
> {
>-    return vlc_uri_component_validate(str, "/@:");
>+    const char *scheme;
>+    const char *pathextras;
>+} scheme_pathextras_list[] = {
>+    /* /!\ Alphabetical order /!\ */
>+    { "ftp",    "/@: []{}", },
>+    { "nfs",    "/@: []{}", },
>+    { "sftp",   "/@: []{}", },
>+    { "smb",    "/@ []{}", },
>+    /* if no matches: allow characters specified by the http protocol.
>*/
>+};
>+
>+static int schemecmp(const void *vkey, const void *ventry)
>+{
>+    const struct scheme_pathextras *entry = ventry;
>+    const char *key = vkey;
>+
>+    return strncmp(key, entry->scheme, strlen(entry->scheme));
> }
> 
> int vlc_UrlParse(vlc_url_t *restrict url, const char *str)
>@@ -547,11 +563,27 @@ int vlc_UrlParse(vlc_url_t *restrict url, const
>char *str)
>         url->psz_path = cur;
>     }
> 
>-    if (url->psz_path != NULL &&
>!vlc_uri_path_validate(url->psz_path))
>+    if (url->psz_path != NULL)
>     {
>-        url->psz_path = NULL;
>-        errno = EINVAL;
>-        ret = -1;
>+        /* By default, only these specials characters are allowed in a
>path
>+         * (http protocol) */
>+        const char *pathextras = "/@:";
>+
>+        if (url->psz_protocol != NULL)
>+        {
>+            const struct scheme_pathextras *e =
>+                bsearch(url->psz_protocol, scheme_pathextras_list,
>+                        ARRAY_SIZE(scheme_pathextras_list),
>+                        sizeof(scheme_pathextras_list[0]), schemecmp);
>+            if (e != NULL)
>+                pathextras = e->pathextras;
>+        }
>+        if (!vlc_uri_component_validate(url->psz_path, pathextras))
>+        {
>+            url->psz_path = NULL;
>+            errno = EINVAL;
>+            ret = -1;
>+        }
>     }
> 
>     return ret;
>-- 
>2.11.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

URL, in general, are untrusted input, which MUST be validated before it is used. Otherwise, you will get injection vulnerabilities in call sites, if not worse. Sorry but in this decade, I don't think we can afford to use that kind of patch because it will be abused.

Even filtering by protocol is problematic. For instance, an FTP URL can be routed through an HTTP proxy, so must follow same rules. Also the generic resolver function assume HTTP syntax.

One thing we can do is to "improve" the fixup function to apply context-sensitive (not protocol-dependent!) encoding. This would fix brackets inside filenames in particular. That is more or less what browsers do.
-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.