[PATCH] daemon: detect and reject too-long paths
- From
Jeff King <peff@peff.net>
- Date
- Oct 22, 2016, 04:59 UTC
- Message-ID
- <20161022045938.h3xa3yapzlg427vy@sigill.intra.peff.net>
When we are checking the path via path_ok(), we use some fixed PATH_MAX buffers. We write into them via snprintf(), so there's no possibility of overflow, but it does mean we may silently truncate the path, leading to potentially confusing errors when the partial path does not exist.
We're better off to reject the path explicitly.
Signed-off-by: Jeff King <peff@peff.net> --- Another option would be to switch to strbufs here. That potentially introduces cases where a client can convince us to just keep allocating memory, but I don't think so in practice; the paths and interpolated data items all have to come in 64K pkt-lines, which places a hard limit. This is a much more minimal change, though, and I don't hear anybody complaining about the inability to use large paths.
daemon.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/daemon.c b/daemon.c index 425aad0507..ff0fa583b0 100644 --- a/daemon.c +++ b/daemon.c @@ -160,6 +160,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) { static char rpath[PATH_MAX]; static char interp_path[PATH_MAX]; + size_t rlen; const char *path; const char *dir; @@ -187,8 +188,12 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) namlen = slash - dir; restlen -= namlen; loginfo("userpath <%s>, request <%s>, namlen %d, restlen %d, slash <%s>", user_path, dir, namlen, restlen, slash); - snprintf(rpath, PATH_MAX, "%.*s/%s%.*s", - namlen, dir, user_path, restlen, slash); + rlen = snprintf(rpath, sizeof(rpath), "%.*s/%s%.*s", + namlen, dir, user_path, restlen, slash); + if (rlen >= sizeof(rpath)) { + logerror("user-path too large: %s", rpath); + return NULL; + } dir = rpath; } } @@ -207,7 +212,15 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) strbuf_expand(&expanded_path, interpolated_path, expand_path, &context); - strlcpy(interp_path, expanded_path.buf, PATH_MAX); + + rlen = strlcpy(interp_path, expanded_path.buf, + sizeof(interp_path)); + if (rlen >= sizeof(interp_path)) { + logerror("interpolated path too large: %s", + interp_path); + return NULL; + } + strbuf_release(&expanded_path); loginfo("Interpolated dir '%s'", interp_path); @@ -219,7 +232,11 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) logerror("'%s': Non-absolute path denied (base-path active)", dir); return NULL; } - snprintf(rpath, PATH_MAX, "%s%s", base_path, dir); + rlen = snprintf(rpath, sizeof(rpath), "%s%s", base_path, dir); + if (rlen >= sizeof(rpath)) { + logerror("base-path too large: %s", rpath); + return NULL; + } dir = rpath; }
-- 2.10.1.776.ge0e381e