diff options
| -rw-r--r-- | browse.c | 4 | ||||
| -rw-r--r-- | download.c | 2 | ||||
| -rw-r--r-- | file.c | 39 | ||||
| -rw-r--r-- | file.h | 14 | ||||
| -rw-r--r-- | http.c | 8 | ||||
| -rw-r--r-- | test.c | 34 | ||||
| -rw-r--r-- | upload.c | 1 | ||||
| -rw-r--r-- | url.c | 80 | ||||
| -rw-r--r-- | url.h | 10 |
9 files changed, 166 insertions, 26 deletions
@@ -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", @@ -33,6 +33,7 @@ #include <fcntl.h> #include <limits.h> +#include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> @@ -44,7 +45,6 @@ #include "http.h" #include "str.h" #include "url.h" -#include "util.h" /* * download url = r->pname / DOWNLOAD_URL / file->path @@ -31,6 +31,7 @@ #include <sys/stat.h> #include <limits.h> +#include <stdarg.h> #include <stdlib.h> #include <string.h> @@ -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; +} @@ -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 */ @@ -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); @@ -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 * @@ -88,10 +91,41 @@ test_upload_post() } 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; } @@ -32,6 +32,7 @@ #include <kcgi.h> #include <kcgihtml.h> #include <limits.h> +#include <stdio.h> #include <string.h> #include "browse.h" @@ -28,13 +28,18 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include <sys/types.h> + #include <assert.h> #include <limits.h> #include <stdarg.h> +#include <stdint.h> #include <stdlib.h> +#include <kcgi.h> #include <string.h> #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; } @@ -32,7 +32,6 @@ #define URL_H #include <stdbool.h> -#include <stdio.h> /* * 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. |