From 19e47daa9c1fb55f2cb9e639d5a6ef16b5bc4e88 Mon Sep 17 00:00:00 2001 From: Vincent Douillet Date: Wed, 11 Sep 2024 11:20:31 +0200 Subject: url: fix encoding for use in http context --- browse.c | 4 ++-- download.c | 2 +- file.c | 39 ++++++++++++++++++++++++++---- file.h | 14 +++++++++++ http.c | 8 ++----- test.c | 34 ++++++++++++++++++++++++++ upload.c | 1 + url.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------- url.h | 10 ++++++-- 9 files changed, 166 insertions(+), 26 deletions(-) diff --git a/browse.c b/browse.c index b233c8b..801b539 100644 --- a/browse.c +++ b/browse.c @@ -114,8 +114,8 @@ build_file_list(struct kreq * r, const struct file * request_dir) || strcmp("..", dir->d_name) == 0) continue; /* build file path */ - file_path_len = url_build(file_path, PATH_MAX, r->path, - dir->d_name, NULL); + file_path_len = path_build(file_path, PATH_MAX, r->path, + dir->d_name, NULL); if (file_path_len == 0 || file_path_len >= PATH_MAX) { kutil_warn(r, NULL, "browse: unable to build file, skipping %s", diff --git a/download.c b/download.c index c18dcc1..008de02 100644 --- a/download.c +++ b/download.c @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -44,7 +45,6 @@ #include "http.h" #include "str.h" #include "url.h" -#include "util.h" /* * download url = r->pname / DOWNLOAD_URL / file->path diff --git a/file.c b/file.c index 24fb557..cd76def 100644 --- a/file.c +++ b/file.c @@ -31,6 +31,7 @@ #include #include +#include #include #include @@ -39,7 +40,6 @@ #include "mime.h" #include "str.h" #include "url.h" -#include "util.h" void file_free(struct file * file) @@ -74,7 +74,7 @@ fill_metadata(struct file * f) f->mime = MIME_BIN; /* check if it is a directory */ - path_len = url_build(path, PATH_MAX, data_dir, f->path, NULL); + path_len = path_build(path, PATH_MAX, data_dir, f->path, NULL); if (path_len == 0 || path_len >= PATH_MAX) return false; @@ -156,10 +156,41 @@ file_get_data_path(const struct file * f, char *data_path, size_t data_path_len, return 0; /* check if it is a directory */ - path_len = url_build(data_path, data_path_len, data_dir, f->path, - append, NULL); + path_len = path_build(data_path, data_path_len, data_dir, f->path, + append, NULL); if (path_len == 0 || path_len >= PATH_MAX) return 0; return path_len; } + +size_t +path_build(char *dst, size_t dst_size,...) +{ + va_list path_list; + const char *path; + size_t w_size; + + dst[0] = '\0'; + w_size = 0; + va_start(path_list, dst_size); + while ((path = va_arg(path_list, char *)) != NULL) { + /* no more space in dst buffer */ + if (w_size >= dst_size) + break; + + if (path[0] == '\0') + continue; + + if (path[0] == '.' && path[1] == '\0') + continue; + + if (path[0] != '/' && w_size > 0 && dst[w_size - 1] != '/') + strlcat(dst, "/", dst_size); + + w_size = strlcat(dst, path, dst_size); + } + + va_end(path_list); + return w_size; +} diff --git a/file.h b/file.h index f6aac76..1ede156 100644 --- a/file.h +++ b/file.h @@ -76,4 +76,18 @@ file_get_basename(const struct file *); size_t file_get_data_path(const struct file *, char *, size_t, char *); +/* + * Build a path from the provided components. + * The first argument is the destination buffer where the path will be written. + * It is required. + * The second argument is the size of the destination buffer. It is required. + * The remaining arguments are the path parts which should end with a NULL + * component. + * Returns the number of characters written to the destination buffer. If it + * is equal to the destination buffer size, it means the buffer was too small + * and the resulting path has been truncated. + */ +size_t +path_build(char *, size_t,...); + #endif /* FILE_H */ diff --git a/http.c b/http.c index 5bad1d8..4dbeff2 100644 --- a/http.c +++ b/http.c @@ -34,7 +34,7 @@ #include "http.h" #include "mime.h" -#include "str.h" +#include "url.h" bool http_open_file(struct kreq * r, enum khttp code, const struct file * f) @@ -49,13 +49,9 @@ http_open_file(struct kreq * r, enum khttp code, const struct file * f) return false; /* file name needs to be url encoded for special chars */ - filename = khttp_urlencode(filename); - if (filename == NULL) + if ((filename = url_encode(filename)) == NULL) return false; - /* but for some reason, spaces should remain spaces... */ - str_replace(filename, '+', ' '); - khttp_head(r, kresps[KRESP_STATUS], "%s", khttps[code]); khttp_head(r, kresps[KRESP_CONTENT_DISPOSITION], "attachment;filename=\"%s\"", filename); diff --git a/test.c b/test.c index b3cf87a..fe1703a 100644 --- a/test.c +++ b/test.c @@ -4,12 +4,15 @@ #include "browse.h" #include "http.h" #include "upload.h" +#include "url.h" /* minunit, see https://jera.com/techinfo/jtns/jtn002 */ #define mu_assert(message, test) do { if (!(test)) return message; } while (0) #define mu_run_test(test) do { char* message = test(); tests_run++; \ if(message) return message; } while (0) +#define BUF_SZ 1000 + int tests_run; static char * @@ -87,11 +90,42 @@ test_upload_post() return 0; } +static char * +test_url_build() +{ + char dst[BUF_SZ], *expected; + size_t dst_len; + + expected = "/vault/browse"; + dst_len = url_build(dst, BUF_SZ, "/vault/browse", "", NULL); + mu_assert("test_url_build failed", dst_len > 0 && dst_len < BUF_SZ); + mu_assert("test_url_build failed", strncmp(expected, dst, BUF_SZ) == 0); + + expected = "/vault/browse/hello/world"; + dst_len = url_build(dst, BUF_SZ, "/vault/browse", "hello", "world", + NULL); + mu_assert("test_url_build failed", dst_len > 0 && dst_len < BUF_SZ); + mu_assert("test_url_build failed", strncmp(expected, dst, BUF_SZ) == 0); + + expected = "/vault/browse"; + dst_len = url_build(dst, BUF_SZ, "/vault/", "browse/", NULL); + mu_assert("test_url_build failed", dst_len > 0 && dst_len < BUF_SZ); + mu_assert("test_url_build failed", strncmp(expected, dst, BUF_SZ) == 0); + + dst_len = url_build(dst, BUF_SZ, "", "", NULL); + mu_assert("test_url_build failed", dst_len == 0); + + dst_len = url_build(dst, BUF_SZ, NULL); + mu_assert("test_url_build failed", dst_len == 0); + return 0; +} + static char * all_tests() { mu_run_test(test_browse_invalid_traversal); mu_run_test(test_browse_path_too_long); + mu_run_test(test_url_build); mu_run_test(test_upload_post); return 0; } diff --git a/upload.c b/upload.c index 765e694..ed9ec4d 100644 --- a/upload.c +++ b/upload.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include "browse.h" diff --git a/url.c b/url.c index adcc95d..977edea 100644 --- a/url.c +++ b/url.c @@ -28,13 +28,18 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include + #include #include #include +#include #include +#include #include #include "config.h" +#include "str.h" #include "url.h" bool @@ -74,20 +79,35 @@ check_request_path(const char *path, const char *suffix) return true; } +char * +url_encode(const char *str) +{ + char *result; + + if ((result = khttp_urlencode(str)) == NULL) + return NULL; + + str_replace(result, '+', ' '); + return result; +} + size_t -url_build(char *dst, size_t dst_size,...) +url_build(char *dst, size_t dst_sz,...) { va_list path_list; - const char *path; - size_t w_size; + char p [PATH_MAX]; + char *encoded, *path, *slash; + size_t c_len, p_len, w_len; dst[0] = '\0'; - w_size = 0; - va_start(path_list, dst_size); + w_len = 0; + va_start(path_list, dst_sz); while ((path = va_arg(path_list, char *)) != NULL) { /* no more space in dst buffer */ - if (w_size >= dst_size) - break; + if (w_len >= dst_sz) { + w_len = 0; + goto end; + } if (path[0] == '\0') continue; @@ -95,12 +115,50 @@ url_build(char *dst, size_t dst_size,...) if (path[0] == '.' && path[1] == '\0') continue; - if (path[0] != '/' && w_size > 0 && dst[w_size - 1] != '/') - strlcat(dst, "/", dst_size); + while (path[0] != '\0') { + /* no more space in dst buffer */ + if (w_len >= dst_sz) { + w_len = 0; + goto end; + } + + /* skip heading slashes */ + slash = strchr(path, '/'); + if (slash == path) { + path++; + continue; + } + + /* copy string to encode into buffer + * if no slash found, copy everything, otherwise stop + * before the slash */ + c_len = slash == NULL ? strlen(path) : + (size_t) (slash - path); + if (c_len >= PATH_MAX) { + w_len = 0; + goto end; + } + p_len = strlcpy(p, path, c_len + 1); + if (p_len < c_len) { + w_len = 0; + goto end; + } + + /* encode string */ + if ((encoded = url_encode(p)) == NULL) { + w_len = 0; + goto end; + } - w_size = strlcat(dst, path, dst_size); + /* append it to the path */ + strlcat(dst, "/", dst_sz); + w_len = strlcat(dst, encoded, dst_sz); + free(encoded); + path += c_len; + } } +end: va_end(path_list); - return w_size; + return w_len; } diff --git a/url.h b/url.h index e91925b..d1d638b 100644 --- a/url.h +++ b/url.h @@ -32,7 +32,6 @@ #define URL_H #include -#include /* * Checks that the path can be safely processed. Namely, it should not contain @@ -42,13 +41,20 @@ */ bool check_request_path(const char *, const char *); +/* + * Encode a string to use in an HTTP context. + * Returns NULL in case of error, or the encoded string in case of success. + */ +char * +url_encode(const char *); /* * Build an URL from the provided components. * The first argument is the destination buffer where the URL will be written. * It is required. * The second argument is the size of the destination buffer. It is required. - * The remaining arguments are the url parts + * The remaining arguments are the url parts which should end with a NULL + * component. * Returns the number of characters written to the destination buffer. If it * is equal to the destination buffer size, it means the buffer was too small * and the resulting URL has been truncated. -- cgit v1.2.3