libweb: Bump new signature for http_decode_url

The new signature allows callers to distinguish decoding errors from
fatal errors. This is important for slcl to avoid crashing when
ill-formed data is received from a client.

.gitignore

@@ -2,4 +2,4 @@ build/
 slcl
 *.o
 *.d
-./Makefile
+Makefile

CMakeLists.txt

@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.13)
-project(slcl LANGUAGES C VERSION 0.2.0)
+project(slcl C VERSION 0.1.0)
 add_executable(${PROJECT_NAME}
     auth.c
     base64.c
@@ -13,7 +13,7 @@ add_executable(${PROJECT_NAME}
 target_compile_options(${PROJECT_NAME} PRIVATE -Wall)
 target_compile_definitions(${PROJECT_NAME} PRIVATE _FILE_OFFSET_BITS=64)
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_LIST_DIR}/cmake)
-find_package(web 0.3.0)
+find_package(web 0.1.0)
 
 if(WEB_FOUND)
     find_package(dynstr 0.1.0)

README.md

@@ -59,13 +59,13 @@ to `slcl`. If required, encryption should be done before uploading e.g.: using
 #### Mandatory packages
 
 ```sh
-sudo apt install build-essential libcjson-dev libssl-dev m4 jq
+sudo apt install build-essential libcjson-dev libssl-dev
 ```
 
 #### Optional packages
 
 ```sh
-sudo apt install cmake
+sudo apt install cmake xxd jq
 ```
 
 ## How to use
@@ -90,8 +90,9 @@ $ make
 #### CMake
 
 ```sh
-$ cmake -B build
-$ cmake --build build/
+$ mkdir build/
+$ cmake ..
+$ cmake --build .
 ```
 
 ### Setting up

configure

@@ -6,7 +6,7 @@ default_prefix=/usr/local
 prefix=$default_prefix
 default_CC='c99'
 # FILE_OFFSET_BITS=64 is required for large file support on 32-bit platforms.
-default_CFLAGS='-O1 -g -D_FILE_OFFSET_BITS=64 -Wall -MD'
+default_CFLAGS='-O1 -g -D_FILE_OFFSET_BITS=64 -Wall -MD -MF $(@:.o=.d)'
 default_LDFLAGS="-lcjson -lssl -lm -lcrypto"
 
 CC=${CC:-$default_CC}
@@ -130,7 +130,7 @@ fi
 if [ $in_tree_libweb -ne 0 ]
 then
 cat <<"EOF" >> $F
-LIBWEB = libweb/libweb.a
+LIBWEB = libweb/libslweb.a
 $(PROJECT): $(LIBWEB)
 $(LIBWEB): FORCE
 	+cd libweb && $(MAKE) CC=$(CC)

libweb

@@ -1 +1 @@
-Subproject commit b4930f72bb9026c5a0871f4fa4cabe20cb0e6a9f
+Subproject commit 2fce1f4b081b645f33692866a5618bdabe4d32c2

main.c

@@ -393,24 +393,14 @@ end:
 
 static bool path_isrel(const char *const path)
 {
-    if (!strcmp(path, "..")
-        || !strcmp(path, ".")
-        || !strncmp(path, "./", strlen("./"))
-        || !strncmp(path, "../", strlen("../"))
-        || strstr(path, "/./")
-        || strstr(path, "/../"))
+    if (!strcmp(path, "..") || !strcmp(path, ".") || strstr(path, "/../"))
         return true;
 
-    static const char *const suffixes[] = {"/.", "/.."};
+    static const char suffix[] = "/..";
+    const size_t n = strlen(path), sn = strlen(suffix);
 
-    for (size_t i = 0; i < sizeof suffixes / sizeof *suffixes; i++)
-    {
-        const char *const suffix = suffixes[i];
-        const size_t n = strlen(path), sn = strlen(suffix);
-
-        if (n >= sn && !strcmp(path + n - sn, suffix))
-            return true;
-    }
+    if (n >= sn && !strcmp(path + n - sn, suffix))
+        return true;
 
     return false;
 }
@@ -435,8 +425,7 @@ static int getpublic(const struct http_payload *const p,
 {
     int ret = -1;
     struct auth *const a = user;
-    const char *const adir = auth_dir(a),
-        *const file = p->resource + strlen("/public/");
+    const char *const adir = auth_dir(a);
     struct dynstr d;
 
     dynstr_init(&d);
@@ -446,13 +435,6 @@ static int getpublic(const struct http_payload *const p,
         fprintf(stderr, "%s: auth_dir failed\n", __func__);
         goto end;
     }
-    else if (!*file || filename_invalid(file))
-    {
-        fprintf(stderr, "%s: invalid filename %s\n",
-            __func__, p->resource);
-        ret = page_forbidden(r);
-        goto end;
-    }
     else if (path_invalid(p->resource))
     {
         fprintf(stderr, "%s: illegal relative path %s\n",
@@ -911,16 +893,7 @@ static int check_length(const unsigned long long len,
     bool has_quota;
     unsigned long long quota;
 
-    if (auth_cookie(a, c))
-    {
-        fprintf(stderr, "%s: auth_cookie failed\n", __func__);
-
-        if (page_forbidden(r))
-            return -1;
-
-        return 1;
-    }
-    else if (auth_quota(a, username, &has_quota, &quota))
+    if (auth_quota(a, username, &has_quota, &quota))
     {
         fprintf(stderr, "%s: auth_quota failed\n", __func__);
         return -1;
@@ -1080,23 +1053,23 @@ end:
 static int move_file(const char *const old, const char *const new)
 {
     int ret = -1;
-    const int fd_old = open(old, O_RDONLY),
-        fd_new = open(new, O_WRONLY | O_CREAT, 0600);
+    FILE *const f = fopen(old, "rb");
+    const int fd = open(new, O_WRONLY | O_CREAT, 0600);
     struct stat sb;
 
-    if (fd_old < 0)
+    if (!f)
     {
-        fprintf(stderr, "%s: open(2) fd_old: %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: fopen(3): %s\n", __func__, strerror(errno));
         goto end;
     }
-    else if (fd_new < 0)
+    else if (fd < 0)
     {
         fprintf(stderr, "%s: open(2): %s\n", __func__, strerror(errno));
         goto end;
     }
-    else if (fstat(fd_old, &sb))
+    else if (stat(old, &sb))
     {
-        fprintf(stderr, "%s: fstat(2): %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: stat(2): %s\n", __func__, strerror(errno));
         goto end;
     }
 
@@ -1105,30 +1078,24 @@ static int move_file(const char *const old, const char *const new)
         char buf[BUFSIZ];
         const off_t left = sb.st_size - i;
         const size_t rem = left > sizeof buf ? sizeof buf : left;
-        const ssize_t r = read(fd_old, buf, rem);
+        ssize_t w;
 
-        if (r < 0)
+        if (!fread(buf, rem, 1, f))
         {
-            fprintf(stderr, "%s: read(2): %s\n", __func__, strerror(errno));
+            fprintf(stderr, "%s: fread(3) failed, feof=%d, ferror=%d\n",
+                __func__, feof(f), ferror(f));
             goto end;
         }
-
-        size_t wrem = r;
-        const void *p = buf;
-
-        while (wrem)
+        else if ((w = write(fd, buf, rem)) < 0)
         {
-            const ssize_t w = write(fd_new, p, wrem);
-
-            if (w < 0)
-            {
-                fprintf(stderr, "%s: write(2): %s\n",
-                    __func__, strerror(errno));
-                goto end;
-            }
-
-            p = (const char *)p + w;
-            wrem -= w;
+            fprintf(stderr, "%s: write(2): %s\n", __func__, strerror(errno));
+            goto end;
+        }
+        else if (w != rem)
+        {
+            fprintf(stderr, "%s: write(2): expected to write %zu bytes, "
+                "only %ju written\n", __func__, rem, (intmax_t)w);
+            goto end;
         }
 
         i += rem;
@@ -1137,15 +1104,15 @@ static int move_file(const char *const old, const char *const new)
     ret = 0;
 
 end:
-    if (fd_old >= 0 && close(fd_old))
+    if (fd >= 0 && close(fd))
     {
-        fprintf(stderr, "%s: close(2) fd_old: %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: close(2): %s\n", __func__, strerror(errno));
         ret = -1;
     }
 
-    if (fd_new >= 0 && close(fd_new))
+    if (f && fclose(f))
     {
-        fprintf(stderr, "%s: close(2) fd_new: %s\n", __func__, strerror(errno));
+        fprintf(stderr, "%s: fclose(3): %s\n", __func__, strerror(errno));
         ret = -1;
     }
 
@@ -1164,43 +1131,12 @@ static int rename_or_move(const char *const old, const char *const new)
     return res;
 }
 
-static int check_upload_dir(const char *const dir)
-{
-    struct stat sb;
-
-    if (stat(dir, &sb))
-    {
-        switch (errno)
-        {
-            case ENOENT:
-                /* Fall through. */
-            case ENOTDIR:
-                fprintf(stderr, "%s: cannot upload to non-existing dir %s\n",
-                    __func__, dir);
-                return 1;
-
-            default:
-                fprintf(stderr, "%s: stat(2) %s: %s\n",
-                    __func__, dir, strerror(errno));
-                return -1;
-        }
-    }
-    else if (!S_ISDIR(sb.st_mode))
-    {
-        fprintf(stderr, "%s: %s not a dir\n", __func__, dir);
-        return 1;
-    }
-
-    return 0;
-}
-
 static int upload_file(const struct http_post_file *const f,
     const char *const user, const char *const root, const char *const dir)
 {
     int ret = -1;
-    struct dynstr dird, d;
+    struct dynstr d;
 
-    dynstr_init(&dird);
     dynstr_init(&d);
 
     if (!root)
@@ -1208,21 +1144,9 @@ static int upload_file(const struct http_post_file *const f,
         fprintf(stderr, "%s: auth_dir failed\n", __func__);
         goto end;
     }
-    else if (dynstr_append(&dird, "%s/user/%s/%s", root, user, dir))
+    else if (dynstr_append(&d, "%s/user/%s/%s%s", root, user, dir, f->filename))
     {
-        fprintf(stderr, "%s: dynstr_append dird failed\n", __func__);
-        goto end;
-    }
-    else if ((ret = check_upload_dir(dird.str)))
-    {
-        if (ret < 0)
-            fprintf(stderr, "%s: check_upload_dir failed\n", __func__);
-
-        goto end;
-    }
-    else if (dynstr_append(&d, "%s%s", dird.str, f->filename))
-    {
-        fprintf(stderr, "%s: dynstr_append d failed\n", __func__);
+        fprintf(stderr, "%s: dynstr_append failed\n", __func__);
         goto end;
     }
     else if (rename_or_move(f->tmpname, d.str))
@@ -1234,7 +1158,6 @@ static int upload_file(const struct http_post_file *const f,
     ret = 0;
 
 end:
-    dynstr_free(&dird);
     dynstr_free(&d);
     return ret;
 }
@@ -1334,15 +1257,11 @@ static int upload_files(const struct http_payload *const p,
 
     for (size_t i = 0; i < po->nfiles; i++)
     {
-        const int ret = upload_file(&po->files[i], user, root, dir);
-
-        if (ret < 0)
+        if (upload_file(&po->files[i], user, root, dir))
         {
             fprintf(stderr, "%s: upload_file failed\n", __func__);
             return -1;
         }
-        else if (ret)
-            return page_bad_request(r);
     }
 
     return redirect_to_dir(dir, r);
@@ -1358,7 +1277,7 @@ static int upload(const struct http_payload *const p,
         fprintf(stderr, "%s: auth_cookie failed\n", __func__);
         return page_forbidden(r);
     }
-    else if (p->expect_continue)
+    else if (p->u.post.expect_continue)
     {
         *r = (const struct http_response)
         {
@@ -1379,7 +1298,6 @@ static int createdir(const struct http_payload *const p,
     struct dynstr d, userd;
     struct form *forms = NULL;
     size_t n = 0;
-    char *encurl = NULL;
 
     dynstr_init(&d);
     dynstr_init(&userd);
@@ -1406,7 +1324,7 @@ static int createdir(const struct http_payload *const p,
         goto end;
     }
 
-    const char *name = NULL, *dir = NULL;
+    char *name = NULL, *dir = NULL;
 
     for (size_t i = 0; i < n; i++)
     {
@@ -1493,12 +1411,7 @@ static int createdir(const struct http_payload *const p,
             .status = HTTP_STATUS_SEE_OTHER
         };
 
-        if (!(encurl = http_encode_url(userd.str)))
-        {
-            fprintf(stderr, "%s: http_encode_url failed\n", __func__);
-            goto end;
-        }
-        else if (http_response_add_header(r, "Location", encurl))
+        if (http_response_add_header(r, "Location", userd.str))
         {
             fprintf(stderr, "%s: http_response_add_header failed\n", __func__);
             goto end;
@@ -1511,7 +1424,6 @@ end:
     forms_free(forms, n);
     dynstr_free(&userd);
     dynstr_free(&d);
-    free(encurl);
     return ret;
 }
 
@@ -2126,31 +2038,14 @@ int main(int argc, char *argv[])
     {
         .length = check_length,
         .tmpdir = tmpdir,
-        .user = a,
-        .post =
-        {
-            /* Arbitrary limit. */
-            .max_files = 10000,
-            /* File upload only requires one pair. */
-            .max_pairs = 1
-        }
+        .user = a
     };
 
-    unsigned short outport;
-
     if (!(h = handler_alloc(&cfg))
         || add_urls(h, a)
-        || handler_listen(h, port, &outport))
+        || handler_listen(h, port))
         goto end;
 
-    printf("Listening on port %hu\n", outport);
-
-    if (handler_loop(h))
-    {
-        fprintf(stderr, "%s: handler_loop failed\n", __func__);
-        goto end;
-    }
-
     ret = EXIT_SUCCESS;
 
 end:

page.c

@@ -108,7 +108,6 @@ static int prepare_name(struct html_node *const n, struct stat *const sb,
     struct html_node *a;
     struct dynstr d, dname;
     const char *const sep = S_ISDIR(sb->st_mode) ? "/" : "";
-    char *encurl = NULL;
 
     dynstr_init(&d);
     dynstr_init(&dname);
@@ -123,12 +122,7 @@ static int prepare_name(struct html_node *const n, struct stat *const sb,
         fprintf(stderr, "%s: html_node_add_child failed\n", __func__);
         goto end;
     }
-    else if (!(encurl = http_encode_url(d.str)))
-    {
-        fprintf(stderr, "%s: http_encode_url failed\n", __func__);
-        goto end;
-    }
-    else if (html_node_add_attr(a, "href", encurl))
+    else if (html_node_add_attr(a, "href", d.str))
     {
         fprintf(stderr, "%s: html_node_add_attr href failed\n", __func__);
         goto end;
@@ -149,7 +143,6 @@ static int prepare_name(struct html_node *const n, struct stat *const sb,
 end:
     dynstr_free(&d);
     dynstr_free(&dname);
-    free(encurl);
     return ret;
 }
 
@@ -310,7 +303,6 @@ static int prepare_preview(struct html_node *const n,
     const struct stat *const sb, const char *const dir, const char *const name)
 {
     int ret = -1;
-    char *encurl = NULL;
     struct html_node *a;
     struct dynstr d;
 
@@ -323,22 +315,9 @@ static int prepare_preview(struct html_node *const n,
         fprintf(stderr, "%s: html_node_add_child form failed\n", __func__);
         goto end;
     }
-    else if (dynstr_append(&d, "%s%s", dir, name))
+    else if (dynstr_append(&d, "%s%s?preview=1", dir, name))
     {
-        fprintf(stderr, "%s: dynstr_append d failed\n", __func__);
-        goto end;
-    }
-    else if (!(encurl = http_encode_url(d.str)))
-    {
-        fprintf(stderr, "%s: http_encode_url failed\n", __func__);
-        goto end;
-    }
-
-    dynstr_free(&d);
-
-    if (dynstr_append(&d, "%s?preview=1", encurl))
-    {
-        fprintf(stderr, "%s: dynstr_append encd failed\n", __func__);
+        fprintf(stderr, "%s: dynstr_append failed\n", __func__);
         goto end;
     }
     else if (html_node_add_attr(a, "href", d.str))
@@ -356,7 +335,6 @@ static int prepare_preview(struct html_node *const n,
 
 end:
     dynstr_free(&d);
-    free(encurl);
     return ret;
 }
 
@@ -1294,24 +1272,20 @@ end:
 }
 
 static int serve_file(struct http_response *const r,
-    const struct stat *const sb, const char *const res, const bool preview,
-    const int fd, bool *const fdopened)
+    const struct stat *const sb, const char *const res, const bool preview)
 {
     int ret = -1;
-    FILE *f = NULL;
+    FILE *const f = fopen(res, "rb");
     struct dynstr b, d;
     char *bn;
 
     dynstr_init(&b);
     dynstr_init(&d);
 
-    if (preview)
+    if (!f)
     {
-        if (dynstr_append(&d, "inline"))
-        {
-            fprintf(stderr, "%s: dynstr_append inline failed\n", __func__);
-            goto end;
-        }
+        fprintf(stderr, "%s: fopen(3): %s\n", __func__, strerror(errno));
+        goto end;
     }
     else if (dynstr_append(&b, "%s", res))
     {
@@ -1323,20 +1297,20 @@ static int serve_file(struct http_response *const r,
         fprintf(stderr, "%s: basename(3) failed\n", __func__);
         goto end;
     }
+    else if (preview)
+    {
+        if (dynstr_append(&d, "inline"))
+        {
+            fprintf(stderr, "%s: dynstr_append inline failed\n", __func__);
+            goto end;
+        }
+    }
     else if (dynstr_append(&d, "attachment; filename=\"%s\"", bn))
     {
         fprintf(stderr, "%s: dynstr_append attachment failed\n", __func__);
         goto end;
     }
 
-    if (!(f = fdopen(fd, "rb")))
-    {
-        fprintf(stderr, "%s: fdopen(3): %s\n", __func__, strerror(errno));
-        goto end;
-    }
-
-    *fdopened = true;
-
     *r = (const struct http_response)
     {
         .status = HTTP_STATUS_OK,
@@ -1406,63 +1380,28 @@ static bool preview(const struct page_resource *const pr)
 
 int page_resource(const struct page_resource *const pr)
 {
-    int ret = -1;
     struct stat sb;
-    const int fd = open(pr->res, O_RDONLY);
-    bool fdopened = false;
 
-    if (fd < 0)
+    if (stat(pr->res, &sb))
     {
-        if (errno == ENOENT || errno == ENOTDIR)
-            ret = page_not_found(pr->r);
-        else
-            fprintf(stderr, "%s: open(2) %s: %s\n",
-                __func__, pr->res, strerror(errno));
-
-        goto end;
-    }
-    else if (fstat(fd, &sb))
-    {
-        fprintf(stderr, "%s: fstat(2) %s: %s\n",
+        fprintf(stderr, "%s: stat(2) %s: %s\n",
             __func__, pr->res, strerror(errno));
-        goto end;
+
+        if (errno == ENOENT || errno == ENOTDIR)
+            return page_not_found(pr->r);
+        else
+            return -1;
     }
 
     const mode_t m = sb.st_mode;
 
     if (S_ISDIR(m))
-    {
-        if (list_dir(pr))
-        {
-            fprintf(stderr, "%s: list_dir failed\n", __func__);
-            goto end;
-        }
-    }
+        return list_dir(pr);
     else if (S_ISREG(m))
-    {
-        if (serve_file(pr->r, &sb, pr->res, preview(pr), fd, &fdopened))
-        {
-            fprintf(stderr, "%s: serve_file failed\n", __func__);
-            goto end;
-        }
-    }
-    else
-    {
-        fprintf(stderr, "%s: unexpected st_mode %jo\n", __func__, (intmax_t)m);
-        goto end;
-    }
+        return serve_file(pr->r, &sb, pr->res, preview(pr));
 
-    ret = 0;
-
-end:
-    if (!fdopened && fd >= 0 && close(fd))
-    {
-        fprintf(stderr, "%s: close(2) %s: %s\n",
-            __func__, pr->res, strerror(errno));
-        ret = -1;
-    }
-
-    return ret;
+    fprintf(stderr, "%s: unexpected st_mode %jd\n", __func__, (intmax_t)m);
+    return -1;
 }
 
 static char *resolve_link(const char *const res)
@@ -1505,26 +1444,18 @@ static char *resolve_link(const char *const res)
 int page_public(struct http_response *const r, const char *const res)
 {
     int ret = -1;
-    const int fd = open(res, O_RDONLY);
     struct stat sb;
     char *path = NULL;
-    bool fdopened = false;
 
-    if (fd < 0)
+    if (stat(res, &sb))
     {
-        if (errno == ENOENT)
-            ret = page_not_found(r);
-        else
-            fprintf(stderr, "%s: stat(2) %s: %s\n",
-                __func__, res, strerror(errno));
-
-        goto end;
-    }
-    else if (fstat(fd, &sb))
-    {
-        fprintf(stderr, "%s: fstat(2) %s: %s\n",
+        fprintf(stderr, "%s: stat(2) %s: %s\n",
             __func__, res, strerror(errno));
-        goto end;
+
+        if (errno == ENOENT)
+            return page_not_found(r);
+        else
+            goto end;
     }
 
     const mode_t m = sb.st_mode;
@@ -1539,7 +1470,7 @@ int page_public(struct http_response *const r, const char *const res)
         fprintf(stderr, "%s: resolve_link failed\n", __func__);
         goto end;
     }
-    else if (serve_file(r, &sb, path, false, fd, &fdopened))
+    else if (serve_file(r, &sb, path, false))
     {
         fprintf(stderr, "%s: serve_file failed\n", __func__);
         goto end;
@@ -1548,13 +1479,6 @@ int page_public(struct http_response *const r, const char *const res)
     ret = 0;
 
 end:
-    if (!fdopened && fd >= 0 && close(fd))
-    {
-        fprintf(stderr, "%s: close(2) %s: %s\n",
-            __func__, res, strerror(errno));
-        ret = -1;
-    }
-
     free(path);
     return ret;
 }

usergen

@@ -9,7 +9,7 @@ usage()
 
 to_hex()
 {
-    od -An -t x1 | tr -d ' ' | tr -d '\n'
+    od -An -t x1 | tr -d ' '
 }
 
 to_bin()
@@ -93,5 +93,5 @@ jq ".users += [
     \"quota\": \"$QUOTA\"
 }]" "$DB" > $TMP
 
-mkdir -p "$DIR/user/$USER"
+mkdir "$DIR/user/$USER"
 mv $TMP "$DB"