From 81ef66eaf09b706125362b7bfcc00d2b4e6f0363 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Mon, 15 Mar 2021 16:23:56 -0400 Subject: [PATCH] [multiple] buffer_has_slash_suffix() buffer_has_slash_suffix() buffer_has_pathsep_suffix() --- src/buffer.h | 13 ++++++++++++ src/mod_alias.c | 2 +- src/mod_dirlisting.c | 3 +-- src/mod_indexfile.c | 4 +--- src/mod_webdav.c | 49 ++++++++++++++------------------------------ src/response.c | 11 ++++------ 6 files changed, 35 insertions(+), 47 deletions(-) diff --git a/src/buffer.h b/src/buffer.h index 7743b989..d055c085 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -229,6 +229,12 @@ static inline uint32_t buffer_string_space(const buffer *b); /* maximum length o static inline void buffer_append_slash(buffer *b); /* append '/' no non-empty strings not ending in '/' */ void buffer_append_path_len(buffer * restrict b, const char * restrict a, size_t alen); /* join strings with '/', if '/' not present */ +__attribute_pure__ +static inline int buffer_has_slash_suffix (const buffer * const b); + +__attribute_pure__ +static inline int buffer_has_pathsep_suffix (const buffer * const b); + #define BUFFER_APPEND_STRING_CONST(x, y) \ buffer_append_string_len(x, y, sizeof(y) - 1) @@ -289,5 +295,12 @@ static inline void buffer_reset(buffer *b) { if (b->size > BUFFER_MAX_REUSE_SIZE) buffer_free_ptr(b); } +static inline int buffer_has_slash_suffix (const buffer * const b) { + return (b->used > 1 && b->ptr[b->used-2] == '/'); +} + +static inline int buffer_has_pathsep_suffix (const buffer * const b) { + return (b->used > 1 && b->ptr[b->used-2] == '/'); +} #endif diff --git a/src/mod_alias.c b/src/mod_alias.c index 65a86f1f..d2c67f30 100644 --- a/src/mod_alias.c +++ b/src/mod_alias.c @@ -122,7 +122,7 @@ mod_alias_remap (request_st * const r, const array * const aliases) { /* do not include trailing slash on basedir */ uint32_t basedir_len = buffer_string_length(&r->physical.basedir); - if ('/' == r->physical.basedir.ptr[basedir_len-1]) --basedir_len; + if (buffer_has_pathsep_suffix(&r->physical.basedir)) --basedir_len; const uint32_t path_len = buffer_string_length(&r->physical.path); if (0 == path_len || path_len < basedir_len) return HANDLER_GO_ON; diff --git a/src/mod_dirlisting.c b/src/mod_dirlisting.c index df84c3ad..0f9c2493 100644 --- a/src/mod_dirlisting.c +++ b/src/mod_dirlisting.c @@ -996,8 +996,7 @@ URIHANDLER_FUNC(mod_dirlisting_subrequest) { plugin_data *p = p_d; if (NULL != r->handler_module) return HANDLER_GO_ON; - if (buffer_string_is_empty(&r->uri.path)) return HANDLER_GO_ON; - if (r->uri.path.ptr[buffer_string_length(&r->uri.path) - 1] != '/') return HANDLER_GO_ON; + if (!buffer_has_slash_suffix(&r->uri.path)) return HANDLER_GO_ON; if (!http_method_get_or_head(r->http_method)) return HANDLER_GO_ON; if (buffer_string_is_empty(&r->physical.path)) return HANDLER_GO_ON; diff --git a/src/mod_indexfile.c b/src/mod_indexfile.c index 97d7de30..0dd9efc9 100644 --- a/src/mod_indexfile.c +++ b/src/mod_indexfile.c @@ -87,9 +87,7 @@ URIHANDLER_FUNC(mod_indexfile_subrequest) { plugin_data *p = p_d; if (NULL != r->handler_module) return HANDLER_GO_ON; - - if (buffer_string_is_empty(&r->uri.path)) return HANDLER_GO_ON; - if (r->uri.path.ptr[buffer_string_length(&r->uri.path) - 1] != '/') return HANDLER_GO_ON; + if (!buffer_has_slash_suffix(&r->uri.path)) return HANDLER_GO_ON; mod_indexfile_patch_config(r, p); if (NULL == p->conf.indexfiles) return HANDLER_GO_ON; diff --git a/src/mod_webdav.c b/src/mod_webdav.c index 32386f75..0378e94e 100644 --- a/src/mod_webdav.c +++ b/src/mod_webdav.c @@ -3923,7 +3923,7 @@ mod_webdav_propfind (request_st * const r, const plugin_config * const pconf) return HANDLER_FINISHED; } else if (S_ISDIR(pb.st.st_mode)) { - if (r->physical.path.ptr[r->physical.path.used - 2] != '/') { + if (!buffer_has_pathsep_suffix(&r->physical.path)) { const buffer *vb = http_header_request_get(r, HTTP_HEADER_USER_AGENT, CONST_STR_LEN("User-Agent")); @@ -3947,7 +3947,7 @@ mod_webdav_propfind (request_st * const r, const plugin_config * const pconf) buffer_append_string_len(&r->physical.rel_path,CONST_STR_LEN("/")); } } - else if (r->physical.path.ptr[r->physical.path.used - 2] == '/') { + else if (buffer_has_pathsep_suffix(&r->physical.path)) { http_status_set_error(r, 403); return HANDLER_FINISHED; } @@ -4141,7 +4141,7 @@ mod_webdav_delete (request_st * const r, const plugin_config * const pconf) } if (S_ISDIR(st.st_mode)) { - if (r->physical.path.ptr[r->physical.path.used - 2] != '/') { + if (!buffer_has_pathsep_suffix(&r->physical.path)) { #if 0 /*(issues warning for /usr/bin/litmus copymove test)*/ http_response_redirect_to_directory(r, 308); return HANDLER_FINISHED; /* 308 Permanent Redirect */ @@ -4194,7 +4194,7 @@ mod_webdav_delete (request_st * const r, const plugin_config * const pconf) /* invalidate stat cache of src if DELETE, whether or not successful */ stat_cache_delete_dir(CONST_BUF_LEN(&r->physical.path)); } - else if (r->physical.path.ptr[r->physical.path.used - 2] == '/') + else if (buffer_has_pathsep_suffix(&r->physical.path)) http_status_set_error(r, 403); else { const int status = webdav_delete_file(pconf, &r->physical); @@ -4825,15 +4825,9 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, else { /*(not expected; some other module mucked with path or rel_path)*/ /* unable to perform physical path remap here; * assume doc_root/rel_path and no remapping */ - #ifdef __COVERITY__ - force_assert(2 <= r->physical.doc_root.used); - force_assert(2 <= dst_path->used); - #endif buffer_copy_buffer(dst_path, &r->physical.doc_root); - if (dst_path->ptr[dst_path->used-2] == '/') - --dst_path->used; /* since dst_rel_path begins with '/' */ buffer_append_path_len(dst_path, CONST_BUF_LEN(dst_rel_path)); - if (buffer_string_length(dst_rel_path) >= PATH_MAX) { + if (buffer_string_length(dst_path) >= PATH_MAX) { http_status_set_error(r, 403); /* Forbidden */ return HANDLER_FINISHED; } @@ -4842,7 +4836,7 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, if (r->physical.path.used <= dst_path->used && 0 == memcmp(r->physical.path.ptr, dst_path->ptr, r->physical.path.used-1) - && (r->physical.path.ptr[r->physical.path.used-2] == '/' + && (buffer_has_pathsep_suffix(&r->physical.path) || dst_path->ptr[r->physical.path.used-1] == '/' || dst_path->ptr[r->physical.path.used-1] == '\0')) { /* dst must not be nested under (or same as) src */ @@ -4863,7 +4857,7 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, } if (S_ISDIR(st.st_mode)) { - if (r->physical.path.ptr[r->physical.path.used - 2] != '/') { + if (!buffer_has_pathsep_suffix(&r->physical.path)) { http_response_redirect_to_directory(r, 308); return HANDLER_FINISHED; /* 308 Permanent Redirect */ /* Alternatively, could append '/' to r->physical.path @@ -4872,10 +4866,7 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, } /* ensure Destination paths end with '/' since dst is a collection */ - #ifdef __COVERITY__ - force_assert(2 <= dst_rel_path->used); - #endif - if (dst_rel_path->ptr[dst_rel_path->used - 2] != '/') { + if (!buffer_has_slash_suffix(dst_rel_path)) { buffer_append_slash(dst_rel_path); buffer_append_slash(dst_path); } @@ -4923,12 +4914,6 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, return HANDLER_FINISHED; } - /* ensure destination is not nested in source */ - if (dst_rel_path->ptr[dst_rel_path->used - 2] != '/') { - buffer_append_slash(dst_rel_path); - buffer_append_slash(dst_path); - } - buffer * const ms = chunk_buffer_acquire(); /* multi-status */ if (0 == webdav_copymove_dir(pconf, &r->physical, dst, ms, flags)) { if (r->http_method == HTTP_METHOD_MOVE) @@ -4951,7 +4936,7 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, stat_cache_delete_dir(CONST_BUF_LEN(&r->physical.path)); return HANDLER_FINISHED; } - else if (r->physical.path.ptr[r->physical.path.used - 2] == '/') { + else if (buffer_has_pathsep_suffix(&r->physical.path)) { http_status_set_error(r, 403); /* Forbidden */ return HANDLER_FINISHED; } @@ -4973,12 +4958,9 @@ mod_webdav_copymove_b (request_st * const r, const plugin_config * const pconf, * append basename to physical path * future: might set Content-Location if dst_path does not end '/'*/ if (NULL != (sep = strrchr(r->physical.path.ptr, '/'))) { - #ifdef __COVERITY__ - force_assert(0 != dst_path->used); - #endif size_t len = r->physical.path.used - 1 - (sep - r->physical.path.ptr); - if (dst_path->ptr[dst_path->used-1] == '/') { + if (buffer_has_pathsep_suffix(dst_path)) { ++sep; /*(avoid double-slash in path)*/ --len; } @@ -5098,7 +5080,7 @@ mod_webdav_proppatch (request_st * const r, const plugin_config * const pconf) } if (S_ISDIR(st.st_mode)) { - if (r->physical.path.ptr[r->physical.path.used - 2] != '/') { + if (!buffer_has_pathsep_suffix(&r->physical.path)) { const buffer *vb = http_header_request_get(r, HTTP_HEADER_USER_AGENT, CONST_STR_LEN("User-Agent")); @@ -5123,7 +5105,7 @@ mod_webdav_proppatch (request_st * const r, const plugin_config * const pconf) buffer_append_string_len(&r->physical.rel_path,CONST_STR_LEN("/")); } } - else if (r->physical.path.ptr[r->physical.path.used - 2] == '/') { + else if (buffer_has_pathsep_suffix(&r->physical.path)) { http_status_set_error(r, 403); return HANDLER_FINISHED; } @@ -5479,8 +5461,7 @@ mod_webdav_lock (request_st * const r, const plugin_config * const pconf) * This is desired behavior since collection should be created * with MKCOL, and not via LOCK on an unmapped resource) */ const int fd = - (errno == ENOENT - && r->physical.path.ptr[r->physical.path.used-2] != '/') + (errno == ENOENT && !buffer_has_pathsep_suffix(&r->physical.path)) ? fdevent_open_cloexec(r->physical.path.ptr, 0, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, WEBDAV_FILE_MODE) @@ -5513,7 +5494,7 @@ mod_webdav_lock (request_st * const r, const plugin_config * const pconf) if (created) { } else if (S_ISDIR(st.st_mode)) { - if (r->physical.path.ptr[r->physical.path.used - 2] != '/') { + if (!buffer_has_pathsep_suffix(&r->physical.path)) { /* 308 Permanent Redirect */ http_response_redirect_to_directory(r, 308); break; /* clean up resources and return HANDLER_FINISHED */ @@ -5522,7 +5503,7 @@ mod_webdav_lock (request_st * const r, const plugin_config * const pconf) * response headers, and continue to serve the request */ } } - else if (r->physical.path.ptr[r->physical.path.used - 2] == '/') { + else if (buffer_has_pathsep_suffix(&r->physical.path)) { http_status_set_error(r, 403); /* Forbidden */ break; /* clean up resources and return HANDLER_FINISHED */ } diff --git a/src/response.c b/src/response.c index 460e7727..f1ec040e 100644 --- a/src/response.c +++ b/src/response.c @@ -198,7 +198,6 @@ static handler_t http_response_physical_path_check(request_st * const r) { if (st) { /* file exists */ } else { - char *pathinfo = NULL; switch (errno) { case ENOTDIR: /* PATH_INFO ! :) */ @@ -221,10 +220,11 @@ static handler_t http_response_physical_path_check(request_st * const r) { /* not found, perhaps PATHINFO */ + char *pathinfo; { /*(might check at startup that s->document_root does not end in '/')*/ - size_t len = buffer_string_length(&r->physical.basedir); - if (len > 0 && '/' == r->physical.basedir.ptr[len-1]) --len; + size_t len = buffer_string_length(&r->physical.basedir) + - (buffer_has_pathsep_suffix(&r->physical.basedir)); pathinfo = r->physical.path.ptr + len; if ('/' != *pathinfo) { pathinfo = NULL; @@ -284,11 +284,8 @@ static handler_t http_response_physical_path_check(request_st * const r) { return HANDLER_GO_ON; if (S_ISDIR(st->st_mode)) { - if (r->uri.path.ptr[buffer_string_length(&r->uri.path) - 1] != '/') { - /* redirect to .../ */ - + if (!buffer_has_slash_suffix(&r->uri.path)) { http_response_redirect_to_directory(r, 301); - return HANDLER_FINISHED; } } else {